Cache prefixing logic rewrite, session scoping improvements, tests refactor (#43)

* Run cache tests on all supported drivers

* update ci healthcheck for memcached

* remove memcached healthcheck

* fix typos in test comments, expand internal.md [ci skip]

* add empty line [ci skip]

* switch to using $store->setPrefix()

* add dynamodb

* refactor try-finally to try-catch

* remove unnecessary clearResolvedInstances() call

* add dual Cache:: and cache() assertions

* add apc

* Flush APCu cache in test setup

* Revert "add dual Cache:: and cache() assertions"

This reverts commit a0bab162fbe2dd0d25e7056ceca4fb7ce54efc77.

* phpstan fix

* Add logic for scoping 'file' disks to FilesystemTenancyBootstrapper

* minor changes, add todos

* refactor how the session.connection is used in the DB session bootstrapper

* add session forgery prevention logic to the db session bootstrapper

* only use the fs bootstrapper for file disk in 'cache data is separated' dataset

* minor session scoping test changes

* Add session scoping logic to FilesystemTenancyBootstrapper, correctly update disk roots even with storage_path_tenancy disabled

* Fix code style (php-cs-fixer)

* update docblock

* make not-null check more explicit

* separate bootstrapper tests, fix swapped test names for two tests

* refactor cache bootstrapper tests

* resolve global cache todo

* expand tests: session separation tests, more filesystem separation assertions; change prefix_base-type config keys to templates/formats

* add apc session scoping test, various session separation bugfixes

* phpstan + minor logic fixes

* prefix_format -> prefix

* fix database session separation test

* revert composer.json changes, update laravel dependencies to expected next release

* only run session scoping logic in cache bootstrapper for redis, memcached, dynamodb, apc; update gitattributes

* tenancy.central_domains -> tenancy.identification.central_domains

* db session separation test: add datasets

---------

Co-authored-by: PHP CS Fixer <phpcsfixer@example.com>

src/Bootstrappers/CacheTenancyBootstrapper.php

@@ -5,10 +5,12 @@ declare(strict_types=1);
 namespace Stancl\Tenancy\Bootstrappers;
 
 use Closure;
+use Exception;
 use Illuminate\Cache\CacheManager;
-use Illuminate\Cache\Repository;
+use Illuminate\Contracts\Cache\Store;
 use Illuminate\Contracts\Config\Repository as ConfigRepository;
-use Illuminate\Support\Facades\Cache;
+use Illuminate\Session\CacheBasedSessionHandler;
+use Illuminate\Session\SessionManager;
 use Stancl\Tenancy\Contracts\TenancyBootstrapper;
 use Stancl\Tenancy\Contracts\Tenant;
 
@@ -17,70 +19,136 @@ use Stancl\Tenancy\Contracts\Tenant;
  */
 class CacheTenancyBootstrapper implements TenancyBootstrapper
 {
+    /** @var Closure(Tenant, string): string */
     public static Closure|null $prefixGenerator = null;
 
-    protected string|null $originalPrefix = null;
+    /** @var array<string, string> */
+    protected array $originalPrefixes = [];
 
     public function __construct(
         protected ConfigRepository $config,
-        protected CacheManager $cacheManager,
+        protected CacheManager $cache,
+        protected SessionManager $session,
     ) {}
 
     public function bootstrap(Tenant $tenant): void
     {
-        $this->originalPrefix = $this->config->get('cache.prefix');
+        foreach ($this->getCacheStores() as $name) {
+            $store = $this->cache->driver($name)->getStore();
 
-        $prefix = $this->generatePrefix($tenant);
+            $this->originalPrefixes[$name] = $store->getPrefix();
+            $this->setCachePrefix($store, $this->generatePrefix($tenant, $name));
+        }
 
-        foreach ($this->config->get('tenancy.cache.stores') as $store) {
-            $this->setCachePrefix($store, $prefix);
+        if ($this->shouldScopeSessions()) {
+            $name = $this->getSessionCacheStoreName();
+            $handler = $this->session->driver()->getHandler();
 
-            // Now that the store uses the passed prefix
-            // Set the configured prefix back to the default one
-            $this->config->set('cache.prefix', $this->originalPrefix);
+            if ($handler instanceof CacheBasedSessionHandler) {
+                // The CacheBasedSessionHandler is constructed with a *clone* of
+                // an existing cache store, so we need to set the prefix separately.
+                $store = $handler->getCache()->getStore();
+
+                // We also don't need to set the original prefix, since the cache store
+                // is implicitly added to the configured cache stores when session scoping
+                // is enabled.
+
+                $this->setCachePrefix($store, $this->generatePrefix($tenant, $name));
+            }
         }
     }
 
     public function revert(): void
     {
-        foreach ($this->config->get('tenancy.cache.stores') as $store) {
-            $this->setCachePrefix($store, $this->originalPrefix);
+        foreach ($this->getCacheStores() as $name) {
+            $store = $this->cache->driver($name)->getStore();
+
+            $this->setCachePrefix($store, $this->originalPrefixes[$name]);
+        }
+
+        if ($this->shouldScopeSessions()) {
+            $name = $this->getSessionCacheStoreName();
+            $handler = $this->session->driver()->getHandler();
+
+            if ($handler instanceof CacheBasedSessionHandler) {
+                $store = $handler->getCache()->getStore();
+
+                $this->setCachePrefix($store, $this->originalPrefixes[$name]);
+            }
         }
     }
 
-    protected function setCachePrefix(string $driver, string|null $prefix): void
+    protected function getSessionCacheStoreName(): string
     {
-        $this->config->set('cache.prefix', $prefix);
-
-        // Refresh driver's store to make the driver use the current prefix
-        $this->refreshStore($driver);
-
-        // It is needed when a call to the facade has been made before bootstrapping tenancy
-        // The facade has its own cache, separate from the container
-        Cache::clearResolvedInstances();
+        return $this->config->get('session.store') ?? $this->config->get('session.driver');
     }
 
-    public function generatePrefix(Tenant $tenant): string
+    protected function shouldScopeSessions(): bool
     {
-        $defaultPrefix = $this->originalPrefix . $this->config->get('tenancy.cache.prefix_base') . $tenant->getTenantKey();
-
-        return static::$prefixGenerator ? (static::$prefixGenerator)($tenant) : $defaultPrefix;
+        // We don't want to scope sessions if:
+        //   1. The user has disabled session scoping via this bootstrapper, AND
+        //   2. The session driver hasn't been instantiated yet (if this is the case,
+        //      it will be instantiated later by cloning an existing cache store
+        //      that will have already been prefixed in this bootstrapper).
+        return $this->config->get('tenancy.cache.scope_sessions', true)
+            && count($this->session->getDrivers()) !== 0;
     }
 
+    /** @return string[] */
+    protected function getCacheStores(): array
+    {
+        $names = $this->config->get('tenancy.cache.stores');
+
+        if (
+            $this->config->get('tenancy.cache.scope_sessions', true) &&
+            in_array($this->config->get('session.driver'), ['redis', 'memcached', 'dynamodb', 'apc'], true)
+        ) {
+            $names[] = $this->getSessionCacheStoreName();
+        }
+
+        $names = array_unique($names);
+
+        return array_filter($names, function ($name) {
+            $store = $this->config->get("cache.stores.{$name}");
+
+            if ($store === null || $store['driver'] === 'file') {
+                return false;
+            }
+
+            if ($store['driver'] === 'array') {
+                throw new Exception('Cache store [' . $name . '] is not supported by this bootstrapper.');
+            }
+
+            return true;
+        });
+    }
+
+    protected function setCachePrefix(Store $store, string|null $prefix): void
+    {
+        if (! method_exists($store, 'setPrefix')) {
+            throw new Exception('Cache store [' . get_class($store) . '] does not support setting a prefix.');
+        }
+
+        $store->setPrefix($prefix);
+    }
+
+    public function generatePrefix(Tenant $tenant, string $store): string
+    {
+        return static::$prefixGenerator
+            ? (static::$prefixGenerator)($tenant, $store)
+            : $this->originalPrefixes[$store] . str($this->config->get('tenancy.cache.prefix'))
+                ->replace('%tenant%', (string) $tenant->getTenantKey())->toString();
+    }
+
+    /**
+     * Set a custom prefix generator.
+     *
+     * The first argument is the tenant, the second argument is the cache store name.
+     *
+     * @param Closure(Tenant, string): string $prefixGenerator
+     */
     public static function generatePrefixUsing(Closure $prefixGenerator): void
     {
         static::$prefixGenerator = $prefixGenerator;
     }
-
-    /**
-     * Refresh cache driver's store.
-     */
-    protected function refreshStore(string $driver): void
-    {
-        $newStore = $this->cacheManager->resolve($driver)->getStore();
-        /** @var Repository $repository */
-        $repository = $this->cacheManager->driver($driver);
-
-        $repository->setStore($newStore);
-    }
 }

src/Bootstrappers/DatabaseSessionBootstrapper.php

@@ -14,6 +14,8 @@ use Stancl\Tenancy\Contracts\Tenant;
 /**
  * This resets the database connection used by the database session driver.
  *
+ * It also includes a mechanism to prevent session forgery when SESSION_CONNECTION is specified.
+ *
  * It runs each time tenancy is initialized or ended.
  * That way the session driver always uses the current DB connection.
  */
@@ -25,23 +27,37 @@ class DatabaseSessionBootstrapper implements TenancyBootstrapper
         protected SessionManager $session,
     ) {}
 
+    protected string|null $originalConnection = null;
+
     public function bootstrap(Tenant $tenant): void
     {
+        $this->originalConnection = $this->config->get('session.connection');
+
         if ($this->config->get('session.driver') === 'database') {
-            $this->resetDatabaseHandler();
+            // At first, this bootstrapper runs before the StartSession middleware, so
+            // changing the session.connection here will affect what connection the session
+            // driver will use. This is helpful to override the SESSION_CONNECTION that might
+            // otherwise allow for session forgery in the tenant context.
+            $this->config->set('session.connection', 'tenant');
+
+            $this->resetDatabaseHandler('tenant');
         }
     }
 
     public function revert(): void
     {
         if ($this->config->get('session.driver') === 'database') {
+            $connection = $this->originalConnection ?? config('tenancy.database.central_connection');
+
             // When ending tenancy, this runs *before* the DatabaseTenancyBootstrapper, so DB tenancy
             // is still bootstrapped. For that reason, we have to explicitly use the central connection
-            $this->resetDatabaseHandler(config('tenancy.database.central_connection'));
+            // instead of null for the default connection.
+            $this->config->set('session.connection', $connection);
+            $this->resetDatabaseHandler($connection);
         }
     }
 
-    protected function resetDatabaseHandler(string $defaultConnection = null): void
+    protected function resetDatabaseHandler(string $connection): void
     {
         $sessionDrivers = $this->session->getDrivers();
 
@@ -49,15 +65,12 @@ class DatabaseSessionBootstrapper implements TenancyBootstrapper
             /** @var \Illuminate\Session\Store $databaseDriver */
             $databaseDriver = $sessionDrivers['database'];
 
-            $databaseDriver->setHandler($this->createDatabaseHandler($defaultConnection));
+            $databaseDriver->setHandler($this->createDatabaseHandler($connection));
         }
     }
 
-    protected function createDatabaseHandler(string $defaultConnection = null): DatabaseSessionHandler
+    protected function createDatabaseHandler(string $connection): DatabaseSessionHandler
     {
-        // Typically returns null, so this falls back to the default DB connection
-        $connection = $this->config->get('session.connection') ?? $defaultConnection;
-
         // Based on SessionManager::createDatabaseDriver
         return new DatabaseSessionHandler(
             $this->container->make('db')->connection($connection),

src/Bootstrappers/FilesystemTenancyBootstrapper.php

@@ -6,6 +6,7 @@ namespace Stancl\Tenancy\Bootstrappers;
 
 use Illuminate\Foundation\Application;
 use Illuminate\Routing\UrlGenerator;
+use Illuminate\Session\FileSessionHandler;
 use Illuminate\Support\Facades\Storage;
 use Stancl\Tenancy\Contracts\TenancyBootstrapper;
 use Stancl\Tenancy\Contracts\Tenant;
@@ -37,6 +38,9 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper
         $this->storagePath($suffix);
         $this->assetHelper($suffix);
         $this->forgetDisks();
+        $this->scopeCache($suffix);
+        $this->scopeSessions($suffix);
+        // todo@docs update fs docs
 
         foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) {
             $this->diskRoot($disk, $tenant);
@@ -55,6 +59,8 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper
         $this->storagePath(false);
         $this->assetHelper(false);
         $this->forgetDisks();
+        $this->scopeCache(false);
+        $this->scopeSessions(false);
 
         foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) {
             $this->diskRoot($disk, false);
@@ -76,10 +82,15 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper
         if ($suffix === false) {
             $this->app->useStoragePath($this->originalStoragePath);
         } else {
-            $this->app->useStoragePath($this->originalStoragePath . "/{$suffix}");
+            $this->app->useStoragePath($this->tenantStoragePath($suffix));
         }
     }
 
+    protected function tenantStoragePath(string $suffix): string
+    {
+        return $this->originalStoragePath . "/{$suffix}";
+    }
+
     protected function assetHelper(string|false $suffix): void
     {
         if (! $this->app['config']['tenancy.filesystem.asset_helper_tenancy']) {
@@ -125,7 +136,7 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper
             // This is executed if the disk is in tenancy.filesystem.disks AND has a root_override
             // This behavior is used for local disks.
             $newRoot = str($override)
-                ->replace('%storage_path%', storage_path())
+                ->replace('%storage_path%', $this->tenantStoragePath($suffix))
                 ->replace('%original_storage_path%', $this->originalStoragePath)
                 ->replace('%tenant%', (string) $tenant->getTenantKey())
                 ->toString();
@@ -156,4 +167,70 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper
             $this->app['config']["filesystems.disks.{$disk}.url"] = url($override);
         }
     }
+
+    public function scopeCache(string|false $suffix): void
+    {
+        if (! $this->app['config']['tenancy.filesystem.scope_cache']) {
+            return;
+        }
+
+        $storagePath = $suffix
+            ? $this->tenantStoragePath($suffix)
+            : $this->originalStoragePath;
+
+        $stores = array_filter($this->app['config']['tenancy.cache.stores'], function ($name) {
+            $store = $this->app['config']["cache.stores.{$name}"];
+
+            if ($store === null) {
+                return false;
+            }
+
+            return $store['driver'] === 'file';
+        });
+
+        foreach ($stores as $name) {
+            $path = $storagePath . '/framework/cache/data';
+            $this->app['config']["cache.stores.{$name}.path"] = $path;
+            $this->app['config']["cache.stores.{$name}.lock_path"] = $path;
+
+            /** @var \Illuminate\Cache\FileStore $store */
+            $store = $this->app['cache']->store($name)->getStore();
+            $store->setDirectory($path);
+            $store->setLockDirectory($path);
+        }
+    }
+
+    public function scopeSessions(string|false $suffix): void
+    {
+        if (! $this->app['config']['tenancy.filesystem.scope_sessions']) {
+            return;
+        }
+
+        $path = $suffix
+            ? $this->tenantStoragePath($suffix) . '/framework/sessions'
+            : $this->originalStoragePath . '/framework/sessions';
+
+        if (! is_dir($path)) {
+            // Create tenant framework/sessions directory if it does not exist
+            mkdir($path, 0755, true);
+        }
+
+        $this->app['config']['session.files'] = $path;
+
+        /** @var \Illuminate\Session\SessionManager $sessionManager */
+        $sessionManager = $this->app['session'];
+
+        // Since this bootstrapper runs much earlier than the StartSession middleware, this doesn't execute
+        // on the average tenant request. It only executes when the context is switched *after* original
+        // middleware initialization.
+        if (isset($sessionManager->getDrivers()['file'])) {
+            $handler = new FileSessionHandler(
+                $this->app->make('files'),
+                $path,
+                $this->app['config']->get('session.lifetime'),
+            );
+
+            $sessionManager->getDrivers()['file']->setHandler($handler);
+        }
+    }
 }

src/Bootstrappers/RedisTenancyBootstrapper.php

@@ -25,7 +25,7 @@ class RedisTenancyBootstrapper implements TenancyBootstrapper
     public function bootstrap(Tenant $tenant): void
     {
         foreach ($this->prefixedConnections() as $connection) {
-            $prefix = $this->config['tenancy.redis.prefix_base'] . $tenant->getTenantKey();
+            $prefix = str($this->config['tenancy.redis.prefix'])->replace('%tenant%', (string) $tenant->getTenantKey())->toString();
             $client = Redis::connection($connection)->client();
 
             /** @var string $originalPrefix */

src/Middleware/InitializeTenancyByDomain.php

@@ -45,7 +45,7 @@ class InitializeTenancyByDomain extends IdentificationMiddleware implements Usab
      */
     public function requestHasTenant(Request $request): bool
     {
-        return ! in_array($this->getDomain($request), config('tenancy.central_domains'));
+        return ! in_array($this->getDomain($request), config('tenancy.identification.central_domains'));
     }
 
     public function getDomain(Request $request): string

src/Middleware/InitializeTenancyByDomainOrSubdomain.php

@@ -50,6 +50,6 @@ class InitializeTenancyByDomainOrSubdomain extends InitializeTenancyBySubdomain
 
     protected function isSubdomain(string $hostname): bool
     {
-        return Str::endsWith($hostname, config('tenancy.central_domains'));
+        return Str::endsWith($hostname, config('tenancy.identification.central_domains'));
     }
 }

src/Middleware/InitializeTenancyBySubdomain.php

@@ -66,9 +66,9 @@ class InitializeTenancyBySubdomain extends InitializeTenancyByDomain
         $isIpAddress = count(array_filter($parts, 'is_numeric')) === count($parts);
 
         // If we're on localhost or an IP address, then we're not visiting a subdomain.
-        $isACentralDomain = in_array($hostname, config('tenancy.central_domains'), true);
+        $isACentralDomain = in_array($hostname, config('tenancy.identification.central_domains'), true);
         $notADomain = $isLocalhost || $isIpAddress;
-        $thirdPartyDomain = ! Str::endsWith($hostname, config('tenancy.central_domains'));
+        $thirdPartyDomain = ! Str::endsWith($hostname, config('tenancy.identification.central_domains'));
 
         if ($isACentralDomain || $notADomain || $thirdPartyDomain) {
             return new NotASubdomainException($hostname);

src/Middleware/PreventAccessFromUnwantedDomains.php

@@ -51,20 +51,20 @@ class PreventAccessFromUnwantedDomains
     protected function accessingTenantRouteFromCentralDomain(Request $request, Route $route): bool
     {
         return tenancy()->getRouteMode($route) === RouteMode::TENANT // Current route's middleware context is tenant
-            && $this->isCentralDomain($request); // The request comes from a domain that IS present in the configured `tenancy.central_domains`
+            && $this->isCentralDomain($request); // The request comes from a domain that IS present in the configured `tenancy.identification.central_domains`
     }
 
     protected function accessingCentralRouteFromTenantDomain(Request $request, Route $route): bool
     {
         return tenancy()->getRouteMode($route) === RouteMode::CENTRAL // Current route's middleware context is central
-            && ! $this->isCentralDomain($request); // The request comes from a domain that ISN'T present in the configured `tenancy.central_domains`
+            && ! $this->isCentralDomain($request); // The request comes from a domain that ISN'T present in the configured `tenancy.identification.central_domains`
     }
 
     /**
-     * Check if the request's host name is present in the configured `tenancy.central_domains`.
+     * Check if the request's host name is present in the configured `tenancy.identification.central_domains`.
      */
     protected function isCentralDomain(Request $request): bool
     {
-        return in_array($request->getHost(), config('tenancy.central_domains'), true);
+        return in_array($request->getHost(), config('tenancy.identification.central_domains'), true);
     }
 }

src/Middleware/ScopeSessions.php

@@ -12,6 +12,9 @@ class ScopeSessions
 {
     public static string $tenantIdKey = '_tenant_id';
 
+    /** @var Closure(Request): mixed */
+    public static Closure|null $onFail = null;
+
     /** @return \Illuminate\Http\Response|mixed */
     public function handle(Request $request, Closure $next): mixed
     {
@@ -23,7 +26,9 @@ class ScopeSessions
             $request->session()->put(static::$tenantIdKey, tenant()->getTenantKey());
         } else {
             if ($request->session()->get(static::$tenantIdKey) !== tenant()->getTenantKey()) {
-                abort(403);
+                return static::$onFail !== null
+                    ? (static::$onFail)($request)
+                    : abort(403);
             }
         }
 

src/Tenancy.php

@@ -117,6 +117,16 @@ class Tenancy
         return array_map('app', $resolve($this->tenant));
     }
 
+    /**
+     * Check if a bootstrapper is being used.
+     *
+     * @param class-string<TenancyBootstrapper> $bootstrapper
+     */
+    public function usingBootstrapper(string $bootstrapper): bool
+    {
+        return in_array($bootstrapper, static::getBootstrappers(), true);
+    }
+
     public static function query(): Builder
     {
         return static::model()->query();