Fix StaySignedIn mechanism

application/front/controller/visitor/LoginController.php

@@ -82,12 +82,12 @@ public function login(Request $request, Response $response): Response
             return $this->index($request, $response);
         }
 
-        $this->container->loginManager->handleSuccessfulLogin($this->container->environment);
-
         $cookiePath = $this->container->basePath . '/';
         $expirationTime = $this->saveLongLastingSession($request, $cookiePath);
         $this->renewUserSession($cookiePath, $expirationTime);
 
+        $this->container->loginManager->handleSuccessfulLogin($this->container->environment);
+
         // Force referer from given return URL
         $this->container->environment['HTTP_REFERER'] = $request->getParam('returnurl');
 
@@ -133,7 +133,7 @@ protected function saveLongLastingSession(Request $request, string $cookiePath):
         $this->container->cookieManager->setCookieParameter(
             CookieManager::STAY_SIGNED_IN,
             $this->container->loginManager->getStaySignedInToken(),
-            $expirationTime,
+            $expirationTime,  // This expirationTime is currently never checked by Shaarli
             $cookiePath
         );
 
@@ -142,14 +142,16 @@ protected function saveLongLastingSession(Request $request, string $cookiePath):
 
     protected function renewUserSession(string $cookiePath, int $expirationTime): void
     {
+        // Quoting https://www.php.net/manual/en/features.session.security.management.php
+        // "Session IDs must be regenerated when user privileges are elevated,
+        // such as after authenticating. session_regenerate_id()
+        // must be called prior to setting the authentication information to $_SESSION."
+        $this->container->sessionManager->regenerateId(true);
         // Send cookie with the new expiration date to the browser
-        $this->container->sessionManager->destroy();
         $this->container->sessionManager->cookieParameters(
             $expirationTime,
             $cookiePath,
             $this->container->environment['SERVER_NAME']
         );
-        $this->container->sessionManager->start();
-        $this->container->sessionManager->regenerateId(true);
     }
 }

application/security/LoginManager.php

@@ -106,7 +106,7 @@ public function checkLoginState($clientIpId)
         if ($this->staySignedInToken === $this->cookieManager->getCookieParameter(CookieManager::STAY_SIGNED_IN)) {
             // The user client has a valid stay-signed-in cookie
             // Session information is updated with the current client information
-            $this->sessionManager->storeLoginInfo($clientIpId);
+            $this->sessionManager->storeSessionInfo($clientIpId);
         } elseif (
             $this->sessionManager->hasSessionExpired()
             || $this->sessionManager->hasClientIpChanged($clientIpId)
@@ -154,8 +154,8 @@ public function checkCredentials($clientIpId, $login, $password)
                     || (true === $useLdapLogin && $this->checkCredentialsFromLdap($login, $password))
                 )
             ) {
-                $this->sessionManager->storeLoginInfo($clientIpId);
-                $this->logger->info(format_log('Login successful', $clientIpId));
+                $this->sessionManager->storeSessionInfo($clientIpId);
+                $this->logger->info(format_log('Login successful for user ' . $login, $clientIpId));
 
                 return true;
             }

application/security/SessionManager.php

@@ -2,6 +2,7 @@
 
 namespace Shaarli\Security;
 
+use Psr\Log\LoggerInterface;
 use Shaarli\Config\ConfigManager;
 
 /**
@@ -35,18 +36,22 @@ class SessionManager
     /** @var string */
     protected $savePath;
 
+    /** @var LoggerInterface */
+    protected $logger;
+
     /**
      * Constructor
      *
      * @param array         $session  The $_SESSION array (reference)
      * @param ConfigManager $conf     ConfigManager instance
      * @param string        $savePath Session save path returned by builtin function session_save_path()
      */
-    public function __construct(&$session, $conf, string $savePath)
+    public function __construct($conf, $logger, &$session, string $savePath)
     {
         $this->session = &$session;
         $this->conf = $conf;
         $this->savePath = $savePath;
+        $this->logger = $logger;
     }
 
     /**
@@ -61,6 +66,10 @@ public function initialize(): void
         if (!isset($this->session['LINKS_PER_PAGE'])) {
             $this->session['LINKS_PER_PAGE'] = $this->conf->get('general.links_per_page', 20);
         }
+        if (!empty($this->session['expires_on']) && $this->session['expires_on'] >= (time() + self::$SHORT_TIMEOUT)) {
+            // We deduce from the session-stored 'expires_on' value if we are currently in a staySignedIn state:
+            $this->setStaySignedIn(true);
+        }
     }
 
     /**
@@ -135,11 +144,11 @@ public static function checkId($sessionId)
     }
 
     /**
-     * Store user login information after a successful login
+     * Store user login information on every page load when logged-in
      *
      * @param string $clientIpId Client IP address identifier
      */
-    public function storeLoginInfo($clientIpId)
+    public function storeSessionInfo($clientIpId)
     {
         $this->session['ip'] = $clientIpId;
         $this->session['username'] = $this->conf->get('credentials.login');
@@ -164,15 +173,26 @@ public function extendSession()
      *
      * @return int New session expiration time
      */
-    protected function extendTimeValidityBy($duration)
+    protected function extendTimeValidityBy($durationSecs)
     {
-        $expirationTime = time() + $duration;
+        $phpSessionLifetimeMin = $this->getPhpSessionLifetimeMin();
+        if ($durationSecs > $phpSessionLifetimeMin * 60) {
+            $this->logger->warning("PHP session lifetime (session.gc_maxlifetime=" . $phpSessionLifetimeMin . "min)"
+                . " is lower than Shaarli session duration (" . $durationSecs / 60 . "min)");
+        }
+        $expirationTime = time() + $durationSecs;
         $this->session['expires_on'] = $expirationTime;
         return $expirationTime;
     }
 
+    protected function getPhpSessionLifetimeMin()
+    {
+        return intval(ini_get("session.gc_maxlifetime"));
+    }
+
     /**
      * Logout a user by unsetting all login information
+     * Currently called on every page if user is not logged-in!
      *
      * See:
      * - https://secure.php.net/manual/en/function.setcookie.php
@@ -197,6 +217,7 @@ public function logout()
     public function hasSessionExpired()
     {
         if (empty($this->session['expires_on'])) {
+            // This is the case if the visitor is simply not logged in
             return true;
         }
         if (time() >= $this->session['expires_on']) {
@@ -306,4 +327,12 @@ public function regenerateId(bool $deleteOldSession = false): bool
     {
         return session_regenerate_id($deleteOldSession);
     }
+
+    /*
+     * Useful for debugging, to get the current state
+     */
+    public function getStaySignedIn(): bool
+    {
+        return $this->staySignedIn;
+    }
 }

doc/md/Server-configuration.md

@@ -74,6 +74,14 @@ Some [plugins](Plugins.md) may require additional configuration.
 - [PHP 5 Changelog](https://www.php.net/ChangeLog-5.php)
 - [PHP: Bugs](https://bugs.php.net/)
 
+#### Configuring session max duration
+
+Shaarli relies on PHP native [session handling](https://www.php.net/manual/en/book.session.php) based on `$SESSION`. A base Shaarli session lasts 1 hour.
+
+Due to this, the session duration is limited by the value of [`session.gc_maxlifetime`](https://www.php.net/manual/en/session.configuration.php#ini.session.gc-maxlifetime) (unit: minutes) set in your PHP configuration (_e.g._ `php.ini`). Its default value is `24min`.
+
+If you want to benefit from Shaarli _stay-signed-in_ feature that only terminates your session after one year of inactivity, you should set `session.gc_maxlifetime` to `525600` (1 year).
+
 
 ## SSL/TLS (HTTPS)
 

index.php

@@ -63,7 +63,7 @@
     !$conf->get('dev.debug') ? LogLevel::INFO : LogLevel::DEBUG,
     ['filename' => basename($conf->get('resource.log'))]
 );
-$sessionManager = new SessionManager($_SESSION, $conf, session_save_path());
+$sessionManager = new SessionManager($conf, $logger, $_SESSION, session_save_path());
 $sessionManager->initialize();
 $cookieManager = new CookieManager($_COOKIE);
 $banManager = new BanManager(

tests/front/controller/visitor/LoginControllerTest.php

@@ -201,14 +201,11 @@ public function testProcessLoginWithValidParameters(): void
         $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8)));
 
         $this->container->sessionManager->expects(static::never())->method('extendSession');
-        $this->container->sessionManager->expects(static::once())->method('destroy');
+        $this->container->sessionManager->expects(static::once())->method('regenerateId')->with(true);
         $this->container->sessionManager
             ->expects(static::once())
             ->method('cookieParameters')
-            ->with(0, '/subfolder/', 'shaarli')
-        ;
-        $this->container->sessionManager->expects(static::once())->method('start');
-        $this->container->sessionManager->expects(static::once())->method('regenerateId')->with(true);
+            ->with(0, '/subfolder/', 'shaarli');
 
         $result = $this->controller->login($request, $response);
 
@@ -268,14 +265,11 @@ public function testProcessLoginLongLastingSession(): void
         $this->container->loginManager->expects(static::once())->method('checkCredentials')->willReturn(true);
         $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8)));
 
-        $this->container->sessionManager->expects(static::once())->method('destroy');
+        $this->container->sessionManager->expects(static::once())->method('regenerateId')->with(true);
         $this->container->sessionManager
             ->expects(static::once())
             ->method('cookieParameters')
-            ->with(42, '/subfolder/', 'shaarli')
-        ;
-        $this->container->sessionManager->expects(static::once())->method('start');
-        $this->container->sessionManager->expects(static::once())->method('regenerateId')->with(true);
+            ->with(42, '/subfolder/', 'shaarli');
         $this->container->sessionManager->expects(static::once())->method('extendSession')->willReturn(42);
 
         $this->container->cookieManager = $this->createMock(CookieManager::class);

tests/security/LoginManagerTest.php

@@ -95,14 +95,15 @@ protected function setUp(): void
         $this->cookieManager->method('getCookieParameter')->willReturnCallback(function (string $key) {
             return $this->cookie[$key] ?? null;
         });
-        $this->sessionManager = new SessionManager($this->session, $this->configManager, 'session_path');
+        $mockLogger = $this->createMock(LoggerInterface::class);
+        $this->sessionManager = new SessionManager($this->configManager, $mockLogger, $this->session, 'session_path');
         $this->banManager = $this->createMock(BanManager::class);
         $this->loginManager = new LoginManager(
             $this->configManager,
             $this->sessionManager,
             $this->cookieManager,
             $this->banManager,
-            $this->createMock(LoggerInterface::class)
+            $mockLogger
         );
         $this->server['REMOTE_ADDR'] = $this->ipAddr;
     }

tests/security/SessionManagerTest.php

@@ -2,6 +2,7 @@
 
 namespace Shaarli\Security;
 
+use Psr\Log\LoggerInterface;
 use Shaarli\FakeConfigManager;
 use Shaarli\TestCase;
 use Shaarli\Tests\Utils\ReferenceSessionIdHashes;
@@ -41,8 +42,9 @@ protected function setUp(): void
             'credentials.salt' => 'salt',
             'security.session_protection_disabled' => false,
         ]);
+        $mockLogger = $this->createMock(LoggerInterface::class);
         $this->session = [];
-        $this->sessionManager = new SessionManager($this->session, $this->conf, 'session_path');
+        $this->sessionManager = new SessionManager($this->conf, $mockLogger, $this->session, 'session_path');
     }
 
     /**
@@ -67,7 +69,8 @@ public function testCheckToken()
                 $token => 1,
             ],
         ];
-        $sessionManager = new SessionManager($session, $this->conf, 'session_path');
+        $mockLogger = $this->createMock(LoggerInterface::class);
+        $sessionManager = new SessionManager($this->conf, $mockLogger, $session, 'session_path');
 
         // check and destroy the token
         $this->assertTrue($sessionManager->checkToken($token));
@@ -157,9 +160,9 @@ public function testIsSessionIdInvalid()
     /**
      * Store login information after a successful login
      */
-    public function testStoreLoginInfo()
+    public function testStoreSessionInfo()
     {
-        $this->sessionManager->storeLoginInfo('ip_id');
+        $this->sessionManager->storeSessionInfo('ip_id');
 
         $this->assertGreaterThan(time(), $this->session['expires_on']);
         $this->assertEquals('ip_id', $this->session['ip']);