From b77421e933188bfd35bcb0ff1a96f330df0e3dbc Mon Sep 17 00:00:00 2001
From: Sami Mazouz <ilyasmazouz@gmail.com>
Date: Mon, 22 Aug 2022 18:46:31 +0100
Subject: [PATCH 01/12] feat: add session driver extension API

Signed-off-by: Sami Mazouz <ilyasmazouz@gmail.com>
---
 framework/core/src/Extend/Session.php         |  42 +++++++
 .../src/Foundation/Console/InfoCommand.php    |  21 +++-
 .../core/src/User/SessionDriverInterface.php  |  27 ++++
 framework/core/src/User/SessionManager.php    |  36 ++++++
 .../core/src/User/SessionServiceProvider.php  |  46 +++++--
 .../integration/extenders/SessionTest.php     | 118 ++++++++++++++++++
 .../testing/src/integration/TestCase.php      |   2 +-
 7 files changed, 283 insertions(+), 9 deletions(-)
 create mode 100644 framework/core/src/Extend/Session.php
 create mode 100644 framework/core/src/User/SessionDriverInterface.php
 create mode 100644 framework/core/src/User/SessionManager.php
 create mode 100644 framework/core/tests/integration/extenders/SessionTest.php

diff --git a/framework/core/src/Extend/Session.php b/framework/core/src/Extend/Session.php
new file mode 100644
index 0000000000..83d4af8a13
--- /dev/null
+++ b/framework/core/src/Extend/Session.php
@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Extend;
+
+use Flarum\Extension\Extension;
+use Illuminate\Contracts\Container\Container;
+
+class Session implements ExtenderInterface
+{
+    private $drivers = [];
+
+    /**
+     * Register a new session driver.
+     *
+     * A driver can currently be selected by setting `session.driver` in `config.php`.
+     *
+     * @param string $name: The name of the driver.
+     * @param string $driverClass: The ::class attribute of the driver.
+     *                             Driver must implement `\Flarum\User\SessionDriverInterface`.
+     * @return self
+     */
+    public function driver(string $name, string $driverClass): self
+    {
+        $this->drivers[$name] = $driverClass;
+
+        return $this;
+    }
+
+    public function extend(Container $container, Extension $extension = null)
+    {
+        $container->extend('flarum.session.drivers', function ($drivers) {
+            return array_merge($drivers, $this->drivers);
+        });
+    }
+}
diff --git a/framework/core/src/Foundation/Console/InfoCommand.php b/framework/core/src/Foundation/Console/InfoCommand.php
index 52a49502ce..3f12f0dba1 100644
--- a/framework/core/src/Foundation/Console/InfoCommand.php
+++ b/framework/core/src/Foundation/Console/InfoCommand.php
@@ -14,8 +14,10 @@
 use Flarum\Foundation\Application;
 use Flarum\Foundation\Config;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\User\SessionManager;
 use Illuminate\Contracts\Queue\Queue;
 use Illuminate\Database\ConnectionInterface;
+use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
 use PDO;
 use Symfony\Component\Console\Helper\Table;
@@ -47,18 +49,25 @@ class InfoCommand extends AbstractCommand
      */
     private $queue;
 
+    /**
+     * @var SessionManager
+     */
+    private $session;
+
     public function __construct(
         ExtensionManager $extensions,
         Config $config,
         SettingsRepositoryInterface $settings,
         ConnectionInterface $db,
-        Queue $queue
+        Queue $queue,
+        SessionManager $session
     ) {
         $this->extensions = $extensions;
         $this->config = $config;
         $this->settings = $settings;
         $this->db = $db;
         $this->queue = $queue;
+        $this->session = $session;
 
         parent::__construct();
     }
@@ -92,6 +101,7 @@ protected function fire()
         $this->output->writeln('<info>Base URL:</info> '.$this->config->url());
         $this->output->writeln('<info>Installation path:</info> '.getcwd());
         $this->output->writeln('<info>Queue driver:</info> '.$this->identifyQueueDriver());
+        $this->output->writeln('<info>Session driver:</info> '.$this->identifySessionDriver());
         $this->output->writeln('<info>Mail driver:</info> '.$this->settings->get('mail_driver', 'unknown'));
         $this->output->writeln('<info>Debug mode:</info> '.($this->config->inDebugMode() ? '<error>ON</error>' : 'off'));
 
@@ -168,4 +178,13 @@ private function identifyDatabaseVersion(): string
     {
         return $this->db->getPdo()->getAttribute(PDO::ATTR_SERVER_VERSION);
     }
+
+    private function identifySessionDriver(): string
+    {
+        $defaultDriver = $this->session->getDefaultDriver();
+        $driver = Arr::get($this->config, 'session.driver', $defaultDriver);
+        $this->session->driver($driver);
+
+        return isset($this->session->getDrivers()[$driver]) ? $driver : $defaultDriver;
+    }
 }
diff --git a/framework/core/src/User/SessionDriverInterface.php b/framework/core/src/User/SessionDriverInterface.php
new file mode 100644
index 0000000000..7a0b284f7e
--- /dev/null
+++ b/framework/core/src/User/SessionDriverInterface.php
@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\User;
+
+use Flarum\Foundation\Config;
+use Flarum\Settings\SettingsRepositoryInterface;
+use SessionHandlerInterface;
+
+interface SessionDriverInterface
+{
+    /**
+     * Build a session handler to handle sessions.
+     * Settings and configuration can either be pulled from the Flarum settings repository
+     * or the config.php file.
+     *
+     * @param SettingsRepositoryInterface $settings: An instance of the Flarum settings repository.
+     * @param Config $config: An instance of the wrapper class around `config.php`.
+     */
+    public function build(SettingsRepositoryInterface $settings, Config $config): SessionHandlerInterface;
+}
diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
new file mode 100644
index 0000000000..52b97225c2
--- /dev/null
+++ b/framework/core/src/User/SessionManager.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\User;
+
+use Flarum\Foundation\Config;
+use Illuminate\Contracts\Container\Container;
+use Illuminate\Session\SessionManager as IlluminateSessionManager;
+use Illuminate\Session\Store;
+use SessionHandlerInterface;
+
+class SessionManager extends IlluminateSessionManager
+{
+    protected $flarumConfig;
+
+    public function __construct(Container $container, Config $flarumConfig)
+    {
+        parent::__construct($container);
+
+        $this->flarumConfig = $flarumConfig;
+    }
+
+    public function handler(string $driver = null): SessionHandlerInterface
+    {
+        /** @var Store $driver */
+        $driver = parent::driver($driver ?? $this->flarumConfig['session.driver']);
+
+        return $driver->getHandler();
+    }
+}
diff --git a/framework/core/src/User/SessionServiceProvider.php b/framework/core/src/User/SessionServiceProvider.php
index 2eb0705470..9f9f8741cc 100644
--- a/framework/core/src/User/SessionServiceProvider.php
+++ b/framework/core/src/User/SessionServiceProvider.php
@@ -10,7 +10,9 @@
 namespace Flarum\User;
 
 use Flarum\Foundation\AbstractServiceProvider;
-use Illuminate\Session\FileSessionHandler;
+use Flarum\Foundation\Config;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Container\Container;
 use SessionHandlerInterface;
 
 class SessionServiceProvider extends AbstractServiceProvider
@@ -20,12 +22,42 @@ class SessionServiceProvider extends AbstractServiceProvider
      */
     public function register()
     {
-        $this->container->singleton('session.handler', function ($container) {
-            return new FileSessionHandler(
-                $container['files'],
-                $container['config']['session.files'],
-                $container['config']['session.lifetime']
-            );
+        $this->container->singleton('flarum.session.drivers', function () {
+            return [];
+        });
+
+        $this->container->singleton('session', function (Container $container) {
+            $config = $container->make(Config::class);
+            $manager = new SessionManager($container, $config);
+            $drivers = $container->make('flarum.session.drivers');
+            $settings = $container->make(SettingsRepositoryInterface::class);
+
+            /**
+             * Default to the file driver already defined by Laravel.
+             *
+             * @see \Illuminate\Session\SessionManager::createFileDriver()
+             */
+            $manager->setDefaultDriver('file');
+
+            foreach ($drivers as $driver => $className) {
+                /** @var SessionDriverInterface $driverInstance */
+                $driverInstance = $container->make($className);
+
+                $manager->extend($driver, function () use ($settings, $config, $driverInstance) {
+                    return $driverInstance->build($settings, $config);
+                });
+            }
+
+            return $manager;
+        });
+
+        $this->container->alias('session', SessionManager::class);
+
+        $this->container->singleton('session.handler', function (Container $container): SessionHandlerInterface {
+            /** @var SessionManager $manager */
+            $manager = $container->make('session');
+
+            return $manager->handler();
         });
 
         $this->container->alias('session.handler', SessionHandlerInterface::class);
diff --git a/framework/core/tests/integration/extenders/SessionTest.php b/framework/core/tests/integration/extenders/SessionTest.php
new file mode 100644
index 0000000000..36dd378f1c
--- /dev/null
+++ b/framework/core/tests/integration/extenders/SessionTest.php
@@ -0,0 +1,118 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\integration\extenders;
+
+use Flarum\Extend;
+use Flarum\Foundation\Config;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\Testing\integration\RetrievesAuthorizedUsers;
+use Flarum\Testing\integration\TestCase;
+use Flarum\User\SessionDriverInterface;
+use Illuminate\Session\FileSessionHandler;
+use Illuminate\Session\NullSessionHandler;
+use InvalidArgumentException;
+use SessionHandlerInterface;
+
+class SessionTest extends TestCase
+{
+    use RetrievesAuthorizedUsers;
+
+    /**
+     * @test
+     */
+    public function default_driver_exists_by_default()
+    {
+        $this->expectNotToPerformAssertions();
+        $this->app()->getContainer()->make('session.handler');
+    }
+
+    /**
+     * @test
+     */
+    public function custom_driver_doesnt_exist_by_default()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->app()->getContainer()->make('session')->handler('flarum-acme');
+    }
+
+    /**
+     * @test
+     */
+    public function custom_driver_exists_if_added()
+    {
+        $this->extend((new Extend\Session())->driver('flarum-acme', AcmeSessionDriver::class));
+
+        $handler = $this->app()->getContainer()->make('session')->handler('flarum-acme');
+
+        $this->assertEquals(NullSessionHandler::class, get_class($handler));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_driver_overrides_laravel_defined_drivers_if_added()
+    {
+        $this->extend((new Extend\Session())->driver('redis', AcmeSessionDriver::class));
+
+        $handler = $this->app()->getContainer()->make('session')->handler('redis');
+
+        $this->assertEquals(NullSessionHandler::class, get_class($handler));
+    }
+
+    /**
+     * @test
+     */
+    public function uses_file_driver_if_driver_from_config_file_not_configured()
+    {
+        $this->config('session.driver', null);
+
+        $handler = $this->app()->getContainer()->make('session.handler');
+
+        $this->assertEquals(FileSessionHandler::class, get_class($handler));
+    }
+
+    /**
+     * @test
+     */
+    public function throws_exception_if_configured_driver_from_config_file_unavailable()
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        $this->config('session.driver', 'nevergonnagiveyouup');
+
+        $handler = $this->app()->getContainer()->make('session.handler');
+
+        $this->assertEquals(FileSessionHandler::class, get_class($handler));
+    }
+
+    /**
+     * @test
+     */
+    public function uses_custom_driver_from_config_file_if_configured_and_available()
+    {
+        $this->extend(
+            (new Extend\Session)->driver('flarum-acme', AcmeSessionDriver::class)
+        );
+
+        $this->config('session.driver', 'flarum-acme');
+
+        $handler = $this->app()->getContainer()->make('session.handler');
+
+        $this->assertEquals(NullSessionHandler::class, get_class($handler));
+    }
+}
+
+class AcmeSessionDriver implements SessionDriverInterface
+{
+    public function build(SettingsRepositoryInterface $settings, Config $config): SessionHandlerInterface
+    {
+        return new NullSessionHandler();
+    }
+}
diff --git a/php-packages/testing/src/integration/TestCase.php b/php-packages/testing/src/integration/TestCase.php
index 79a7a7f256..4efcec34ca 100644
--- a/php-packages/testing/src/integration/TestCase.php
+++ b/php-packages/testing/src/integration/TestCase.php
@@ -148,7 +148,7 @@ protected function extension(string ...$extensions)
      */
     protected function config(string $key, $value)
     {
-        $this->config[$key] = $value;
+        Arr::set($this->config, $key, $value);
     }
 
     /**

From 12eb9b971ac4a535f654f21f3ab349d45662c744 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <ilyasmazouz@gmail.com>
Date: Mon, 22 Aug 2022 18:53:13 +0100
Subject: [PATCH 02/12] chore: no need to inject into SessionManager

Signed-off-by: Sami Mazouz <ilyasmazouz@gmail.com>
---
 framework/core/src/User/SessionManager.php         | 14 +++-----------
 framework/core/src/User/SessionServiceProvider.php |  4 ++--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index 52b97225c2..645facf429 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -10,26 +10,18 @@
 namespace Flarum\User;
 
 use Flarum\Foundation\Config;
-use Illuminate\Contracts\Container\Container;
 use Illuminate\Session\SessionManager as IlluminateSessionManager;
 use Illuminate\Session\Store;
 use SessionHandlerInterface;
 
 class SessionManager extends IlluminateSessionManager
 {
-    protected $flarumConfig;
-
-    public function __construct(Container $container, Config $flarumConfig)
-    {
-        parent::__construct($container);
-
-        $this->flarumConfig = $flarumConfig;
-    }
-
     public function handler(string $driver = null): SessionHandlerInterface
     {
+        $config = $this->container->make(Config::class);
+
         /** @var Store $driver */
-        $driver = parent::driver($driver ?? $this->flarumConfig['session.driver']);
+        $driver = parent::driver($driver ?? $config['session.driver']);
 
         return $driver->getHandler();
     }
diff --git a/framework/core/src/User/SessionServiceProvider.php b/framework/core/src/User/SessionServiceProvider.php
index 9f9f8741cc..e0f8023a2b 100644
--- a/framework/core/src/User/SessionServiceProvider.php
+++ b/framework/core/src/User/SessionServiceProvider.php
@@ -27,10 +27,10 @@ public function register()
         });
 
         $this->container->singleton('session', function (Container $container) {
-            $config = $container->make(Config::class);
-            $manager = new SessionManager($container, $config);
+            $manager = new SessionManager($container);
             $drivers = $container->make('flarum.session.drivers');
             $settings = $container->make(SettingsRepositoryInterface::class);
+            $config = $container->make(Config::class);
 
             /**
              * Default to the file driver already defined by Laravel.

From ccb2799cd6f6f86d41d77e4b7f3ae7fc87564b81 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <ilyasmazouz@gmail.com>
Date: Mon, 22 Aug 2022 19:05:56 +0100
Subject: [PATCH 03/12] chore: improve current session driver detection

Signed-off-by: Sami Mazouz <ilyasmazouz@gmail.com>
---
 .../src/Foundation/Console/InfoCommand.php    | 28 +++++++++++++++++--
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/framework/core/src/Foundation/Console/InfoCommand.php b/framework/core/src/Foundation/Console/InfoCommand.php
index 3f12f0dba1..ce7abc3aae 100644
--- a/framework/core/src/Foundation/Console/InfoCommand.php
+++ b/framework/core/src/Foundation/Console/InfoCommand.php
@@ -20,6 +20,7 @@
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
 use PDO;
+use SessionHandlerInterface;
 use Symfony\Component\Console\Helper\Table;
 use Symfony\Component\Console\Helper\TableStyle;
 
@@ -54,13 +55,19 @@ class InfoCommand extends AbstractCommand
      */
     private $session;
 
+    /**
+     * @var SessionHandlerInterface
+     */
+    private $sessionHandler;
+
     public function __construct(
         ExtensionManager $extensions,
         Config $config,
         SettingsRepositoryInterface $settings,
         ConnectionInterface $db,
         Queue $queue,
-        SessionManager $session
+        SessionManager $session,
+        SessionHandlerInterface $sessionHandler
     ) {
         $this->extensions = $extensions;
         $this->config = $config;
@@ -68,6 +75,7 @@ public function __construct(
         $this->db = $db;
         $this->queue = $queue;
         $this->session = $session;
+        $this->sessionHandler = $sessionHandler;
 
         parent::__construct();
     }
@@ -179,12 +187,26 @@ private function identifyDatabaseVersion(): string
         return $this->db->getPdo()->getAttribute(PDO::ATTR_SERVER_VERSION);
     }
 
+    /**
+     * Some extensions/packages might be overriding the session.handler binding.
+     * So we have to check if the driver used is configured or hardcoded.
+     */
     private function identifySessionDriver(): string
     {
         $defaultDriver = $this->session->getDefaultDriver();
         $driver = Arr::get($this->config, 'session.driver', $defaultDriver);
-        $this->session->driver($driver);
+        $this->session->handler($driver);
+        $configuredDriver = isset($this->session->getDrivers()[$driver]) ? $driver : $defaultDriver;
+
+        // Get class name
+        $handlerName = get_class($this->sessionHandler);
+        // Drop the namespace
+        $handlerName = Str::afterLast($handlerName, '\\');
+        // Lowercase the class name
+        $handlerName = strtolower($handlerName);
+        // Drop everything like queue SyncQueue, RedisQueue
+        $handlerName = str_replace('sessionhandler', '', $handlerName);
 
-        return isset($this->session->getDrivers()[$driver]) ? $driver : $defaultDriver;
+        return $configuredDriver !== $handlerName ? "$handlerName <comment>(code override, configured to <options=bold,underscore>$configuredDriver</>)</comment>" : $configuredDriver;
     }
 }

From 8030d11edf3a43e9619089504455e77a000032cf Mon Sep 17 00:00:00 2001
From: Sami Mazouz <ilyasmazouz@gmail.com>
Date: Mon, 22 Aug 2022 19:15:45 +0100
Subject: [PATCH 04/12] chore: silently fallback to default if configured
 driver is unavailable

Signed-off-by: Sami Mazouz <ilyasmazouz@gmail.com>
---
 framework/core/src/User/SessionManager.php          | 13 +++++++++++--
 .../tests/integration/extenders/SessionTest.php     |  6 ++----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index 645facf429..30f104f117 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -12,6 +12,7 @@
 use Flarum\Foundation\Config;
 use Illuminate\Session\SessionManager as IlluminateSessionManager;
 use Illuminate\Session\Store;
+use InvalidArgumentException;
 use SessionHandlerInterface;
 
 class SessionManager extends IlluminateSessionManager
@@ -21,8 +22,16 @@ public function handler(string $driver = null): SessionHandlerInterface
         $config = $this->container->make(Config::class);
 
         /** @var Store $driver */
-        $driver = parent::driver($driver ?? $config['session.driver']);
+        try {
+            $driverInstance = parent::driver($driver ?? $config['session.driver']);
+        } catch (InvalidArgumentException $e) {
+            if (! $driver) {
+                $driverInstance = parent::driver($this->getDefaultDriver());
+            } else {
+                throw $e;
+            }
+        }
 
-        return $driver->getHandler();
+        return $driverInstance->getHandler();
     }
 }
diff --git a/framework/core/tests/integration/extenders/SessionTest.php b/framework/core/tests/integration/extenders/SessionTest.php
index 36dd378f1c..9a78db1666 100644
--- a/framework/core/tests/integration/extenders/SessionTest.php
+++ b/framework/core/tests/integration/extenders/SessionTest.php
@@ -69,7 +69,7 @@ public function custom_driver_overrides_laravel_defined_drivers_if_added()
     /**
      * @test
      */
-    public function uses_file_driver_if_driver_from_config_file_not_configured()
+    public function uses_default_driver_if_driver_from_config_file_not_configured()
     {
         $this->config('session.driver', null);
 
@@ -81,10 +81,8 @@ public function uses_file_driver_if_driver_from_config_file_not_configured()
     /**
      * @test
      */
-    public function throws_exception_if_configured_driver_from_config_file_unavailable()
+    public function uses_default_driver_if_configured_driver_from_config_file_unavailable()
     {
-        $this->expectException(InvalidArgumentException::class);
-
         $this->config('session.driver', 'nevergonnagiveyouup');
 
         $handler = $this->app()->getContainer()->make('session.handler');

From 8bd47b60c08112d716bf0f1cd22b4c07a0b19db4 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <ilyasmazouz@gmail.com>
Date: Tue, 23 Aug 2022 19:04:27 +0100
Subject: [PATCH 05/12] chore: log a critical error if the configured session
 driver is unavailable.

Signed-off-by: Sami Mazouz <ilyasmazouz@gmail.com>
---
 framework/core/src/User/SessionManager.php | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index 30f104f117..8df02a15cc 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -13,6 +13,7 @@
 use Illuminate\Session\SessionManager as IlluminateSessionManager;
 use Illuminate\Session\Store;
 use InvalidArgumentException;
+use Psr\Log\LoggerInterface;
 use SessionHandlerInterface;
 
 class SessionManager extends IlluminateSessionManager
@@ -26,7 +27,14 @@ public function handler(string $driver = null): SessionHandlerInterface
             $driverInstance = parent::driver($driver ?? $config['session.driver']);
         } catch (InvalidArgumentException $e) {
             if (! $driver) {
+                // If we're expecting the default driver, and it's not available,
+                // then we'll fall back to the default driver.
                 $driverInstance = parent::driver($this->getDefaultDriver());
+
+                // But we will log a critical error to the webmaster.
+                $this->container->make(LoggerInterface::class)->critical(
+                    'The default session driver is not available. Please check your configuration.'
+                );
             } else {
                 throw $e;
             }

From 7b7e65e32ffcce932d2ae1e780e58aabba8814f0 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <ilyasmazouz@gmail.com>
Date: Wed, 24 Aug 2022 11:19:54 +0100
Subject: [PATCH 06/12] fix(review): simplify session manager logic

Signed-off-by: Sami Mazouz <ilyasmazouz@gmail.com>
---
 .../src/Foundation/Console/InfoCommand.php    |  2 +-
 framework/core/src/User/SessionManager.php    | 31 ++++++++++---------
 .../integration/extenders/SessionTest.php     | 10 +++---
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/framework/core/src/Foundation/Console/InfoCommand.php b/framework/core/src/Foundation/Console/InfoCommand.php
index ce7abc3aae..6d363e2ae0 100644
--- a/framework/core/src/Foundation/Console/InfoCommand.php
+++ b/framework/core/src/Foundation/Console/InfoCommand.php
@@ -195,7 +195,7 @@ private function identifySessionDriver(): string
     {
         $defaultDriver = $this->session->getDefaultDriver();
         $driver = Arr::get($this->config, 'session.driver', $defaultDriver);
-        $this->session->handler($driver);
+        $this->session->driver($driver);
         $configuredDriver = isset($this->session->getDrivers()[$driver]) ? $driver : $defaultDriver;
 
         // Get class name
diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index 8df02a15cc..55973e2c7a 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -12,32 +12,35 @@
 use Flarum\Foundation\Config;
 use Illuminate\Session\SessionManager as IlluminateSessionManager;
 use Illuminate\Session\Store;
+use Illuminate\Support\Arr;
 use InvalidArgumentException;
 use Psr\Log\LoggerInterface;
 use SessionHandlerInterface;
 
 class SessionManager extends IlluminateSessionManager
 {
-    public function handler(string $driver = null): SessionHandlerInterface
+    /**
+     * Returns the configured session handler.
+     * Picks up the driver from `config.php` using the `session.driver` item.
+     * Falls back to the default driver if the configured one is not available,
+     *  and logs a critical error in that case.
+     */
+    public function handler(): SessionHandlerInterface
     {
         $config = $this->container->make(Config::class);
+        $driverName = Arr::get($config, 'session.driver');
 
-        /** @var Store $driver */
+        /** @var Store $driverInstance */
         try {
-            $driverInstance = parent::driver($driver ?? $config['session.driver']);
+            $driverInstance = parent::driver($driverName);
         } catch (InvalidArgumentException $e) {
-            if (! $driver) {
-                // If we're expecting the default driver, and it's not available,
-                // then we'll fall back to the default driver.
-                $driverInstance = parent::driver($this->getDefaultDriver());
+            $defaultDriverName = $this->getDefaultDriver();
+            $driverInstance = parent::driver($defaultDriverName);
 
-                // But we will log a critical error to the webmaster.
-                $this->container->make(LoggerInterface::class)->critical(
-                    'The default session driver is not available. Please check your configuration.'
-                );
-            } else {
-                throw $e;
-            }
+            // But we will log a critical error to the webmaster.
+            $this->container->make(LoggerInterface::class)->critical(
+                "The configured session driver [$driverName] is not available. Falling back to [$defaultDriverName]. Please check your configuration."
+            );
         }
 
         return $driverInstance->getHandler();
diff --git a/framework/core/tests/integration/extenders/SessionTest.php b/framework/core/tests/integration/extenders/SessionTest.php
index 9a78db1666..82efc54564 100644
--- a/framework/core/tests/integration/extenders/SessionTest.php
+++ b/framework/core/tests/integration/extenders/SessionTest.php
@@ -39,7 +39,7 @@ public function default_driver_exists_by_default()
     public function custom_driver_doesnt_exist_by_default()
     {
         $this->expectException(InvalidArgumentException::class);
-        $this->app()->getContainer()->make('session')->handler('flarum-acme');
+        $this->app()->getContainer()->make('session')->driver('flarum-acme');
     }
 
     /**
@@ -49,9 +49,9 @@ public function custom_driver_exists_if_added()
     {
         $this->extend((new Extend\Session())->driver('flarum-acme', AcmeSessionDriver::class));
 
-        $handler = $this->app()->getContainer()->make('session')->handler('flarum-acme');
+        $driver = $this->app()->getContainer()->make('session')->driver('flarum-acme');
 
-        $this->assertEquals(NullSessionHandler::class, get_class($handler));
+        $this->assertEquals(NullSessionHandler::class, get_class($driver->getHandler()));
     }
 
     /**
@@ -61,9 +61,9 @@ public function custom_driver_overrides_laravel_defined_drivers_if_added()
     {
         $this->extend((new Extend\Session())->driver('redis', AcmeSessionDriver::class));
 
-        $handler = $this->app()->getContainer()->make('session')->handler('redis');
+        $driver = $this->app()->getContainer()->make('session')->driver('redis');
 
-        $this->assertEquals(NullSessionHandler::class, get_class($handler));
+        $this->assertEquals(NullSessionHandler::class, get_class($driver->getHandler()));
     }
 
     /**

From 9bb2fb97bc4474858d3b96d2452837dbdd604899 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Tue, 6 Sep 2022 22:47:21 +0100
Subject: [PATCH 07/12] chore(review): clearer error message

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
---
 framework/core/src/User/SessionManager.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index 55973e2c7a..b032aa88c2 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -39,7 +39,7 @@ public function handler(): SessionHandlerInterface
 
             // But we will log a critical error to the webmaster.
             $this->container->make(LoggerInterface::class)->critical(
-                "The configured session driver [$driverName] is not available. Falling back to [$defaultDriverName]. Please check your configuration."
+                "The configured session driver [$driverName] is not available. Falling back to default [$defaultDriverName]. Please check your configuration."
             );
         }
 

From 6e53566234084e0c206eb4051a583d028808798a Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Tue, 6 Sep 2022 22:47:58 +0100
Subject: [PATCH 08/12] chore(review): improve `InfoCommand` code readability

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
---
 .../src/Foundation/Console/InfoCommand.php    | 36 +++++++++++++++----
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/framework/core/src/Foundation/Console/InfoCommand.php b/framework/core/src/Foundation/Console/InfoCommand.php
index 6d363e2ae0..0010ac4fb7 100644
--- a/framework/core/src/Foundation/Console/InfoCommand.php
+++ b/framework/core/src/Foundation/Console/InfoCommand.php
@@ -19,6 +19,7 @@
 use Illuminate\Database\ConnectionInterface;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
+use InvalidArgumentException;
 use PDO;
 use SessionHandlerInterface;
 use Symfony\Component\Console\Helper\Table;
@@ -188,25 +189,46 @@ private function identifyDatabaseVersion(): string
     }
 
     /**
-     * Some extensions/packages might be overriding the session.handler binding.
-     * So we have to check if the driver used is configured or hardcoded.
+     * Reports on the session driver in use based on three scenarios:
+     *  1. If the configured session driver is valid and in use, it will be returned.
+     *  2. If the configured session driver is invalid, fallback to the default one and mention it.
+     *  3. If the actual used driver (i.e `session.handler`) is different from the current one (configured or default), mention it.
      */
     private function identifySessionDriver(): string
     {
+        /*
+         * Get the configured driver and fallback to the default one.
+         */
         $defaultDriver = $this->session->getDefaultDriver();
-        $driver = Arr::get($this->config, 'session.driver', $defaultDriver);
-        $this->session->driver($driver);
-        $configuredDriver = isset($this->session->getDrivers()[$driver]) ? $driver : $defaultDriver;
+        $configuredDriver = Arr::get($this->config, 'session.driver', $defaultDriver);
+        $driver = $configuredDriver;
+        try {
+            // Try to get the configured driver instance.
+            // Driver instances are created on demand.
+            $this->session->driver($configuredDriver);
+        } catch (InvalidArgumentException $e) {
+            // An exception is thrown if the configured driver is not a valid driver.
+            // So we fallback to the default driver.
+            $driver = $defaultDriver;
+        }
 
+        /*
+         * Get actual driver name from its class name.
+         * And compare that to the current configured driver.
+         */
         // Get class name
         $handlerName = get_class($this->sessionHandler);
         // Drop the namespace
         $handlerName = Str::afterLast($handlerName, '\\');
         // Lowercase the class name
         $handlerName = strtolower($handlerName);
-        // Drop everything like queue SyncQueue, RedisQueue
+        // Drop everything like sessionhandler FileSessionHandler, DatabaseSessionHandler ..etc
         $handlerName = str_replace('sessionhandler', '', $handlerName);
 
-        return $configuredDriver !== $handlerName ? "$handlerName <comment>(code override, configured to <options=bold,underscore>$configuredDriver</>)</comment>" : $configuredDriver;
+        return $driver !== $handlerName
+            ? "$handlerName <comment>(Code override. Configured to <options=bold,underscore>$configuredDriver</>)</comment>"
+            : ($driver !== $configuredDriver
+                ? "$driver <comment>(Fallback default driver. Configured to invalid driver <options=bold,underscore>$configuredDriver</>)</comment>"
+                : $driver);
     }
 }

From f2849460b94343efb2514338055b4c4a3e738519 Mon Sep 17 00:00:00 2001
From: StyleCI Bot <bot@styleci.io>
Date: Tue, 6 Sep 2022 21:48:17 +0000
Subject: [PATCH 09/12] Apply fixes from StyleCI

---
 framework/core/src/Foundation/Console/InfoCommand.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/framework/core/src/Foundation/Console/InfoCommand.php b/framework/core/src/Foundation/Console/InfoCommand.php
index 0010ac4fb7..862d48814d 100644
--- a/framework/core/src/Foundation/Console/InfoCommand.php
+++ b/framework/core/src/Foundation/Console/InfoCommand.php
@@ -202,6 +202,7 @@ private function identifySessionDriver(): string
         $defaultDriver = $this->session->getDefaultDriver();
         $configuredDriver = Arr::get($this->config, 'session.driver', $defaultDriver);
         $driver = $configuredDriver;
+
         try {
             // Try to get the configured driver instance.
             // Driver instances are created on demand.

From a9c62e4cfa0d6e2854a7923c8e7247aab5241a95 Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Tue, 6 Sep 2022 22:50:34 +0100
Subject: [PATCH 10/12] chore(review): improve `InfoCommand` code readability

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
---
 .../core/src/Foundation/Console/InfoCommand.php    | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/framework/core/src/Foundation/Console/InfoCommand.php b/framework/core/src/Foundation/Console/InfoCommand.php
index 862d48814d..31c21ea02a 100644
--- a/framework/core/src/Foundation/Console/InfoCommand.php
+++ b/framework/core/src/Foundation/Console/InfoCommand.php
@@ -226,10 +226,14 @@ private function identifySessionDriver(): string
         // Drop everything like sessionhandler FileSessionHandler, DatabaseSessionHandler ..etc
         $handlerName = str_replace('sessionhandler', '', $handlerName);
 
-        return $driver !== $handlerName
-            ? "$handlerName <comment>(Code override. Configured to <options=bold,underscore>$configuredDriver</>)</comment>"
-            : ($driver !== $configuredDriver
-                ? "$driver <comment>(Fallback default driver. Configured to invalid driver <options=bold,underscore>$configuredDriver</>)</comment>"
-                : $driver);
+        if ($driver !== $handlerName) {
+            return "$handlerName <comment>(Code override. Configured to <options=bold,underscore>$configuredDriver</>)</comment>";
+        }
+
+        if ($driver !== $configuredDriver) {
+            return "$driver <comment>(Fallback default driver. Configured to invalid driver <options=bold,underscore>$configuredDriver</>)</comment>";
+        }
+
+        return $driver;
     }
 }

From 09420864f64598bf9a8d09fc604fa45a3973c1fe Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Wed, 14 Sep 2022 17:54:44 +0100
Subject: [PATCH 11/12] fix: phpstan detected errors

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
---
 framework/core/src/User/SessionManager.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index b032aa88c2..f4048ebabe 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -30,7 +30,6 @@ public function handler(): SessionHandlerInterface
         $config = $this->container->make(Config::class);
         $driverName = Arr::get($config, 'session.driver');
 
-        /** @var Store $driverInstance */
         try {
             $driverInstance = parent::driver($driverName);
         } catch (InvalidArgumentException $e) {

From 7f2a0c010da1de90c9d236c3a3c90faa918f8ada Mon Sep 17 00:00:00 2001
From: StyleCI Bot <bot@styleci.io>
Date: Wed, 14 Sep 2022 16:55:05 +0000
Subject: [PATCH 12/12] Apply fixes from StyleCI

---
 framework/core/src/User/SessionManager.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/framework/core/src/User/SessionManager.php b/framework/core/src/User/SessionManager.php
index f4048ebabe..f550cdc11f 100644
--- a/framework/core/src/User/SessionManager.php
+++ b/framework/core/src/User/SessionManager.php
@@ -11,7 +11,6 @@
 
 use Flarum\Foundation\Config;
 use Illuminate\Session\SessionManager as IlluminateSessionManager;
-use Illuminate\Session\Store;
 use Illuminate\Support\Arr;
 use InvalidArgumentException;
 use Psr\Log\LoggerInterface;