From 8edd7f15886620b07064aa889aea05c5acbc0e58 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Wed, 30 May 2018 02:09:09 +0200
Subject: [PATCH] SessionManager+LoginManager: fix checkLoginState logic

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/security/LoginManager.php   |  2 +-
 application/security/SessionManager.php |  5 ++++-
 tests/security/LoginManagerTest.php     | 15 ++++++++++-----
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index 4946850b..d6784d6d 100644
--- a/application/security/LoginManager.php
+++ b/application/security/LoginManager.php
@@ -95,7 +95,6 @@ class LoginManager
             // The user client has a valid stay-signed-in cookie
             // Session information is updated with the current client information
             $this->sessionManager->storeLoginInfo($clientIpId);
-            $this->isLoggedIn = true;
 
         } elseif ($this->sessionManager->hasSessionExpired()
             || $this->sessionManager->hasClientIpChanged($clientIpId)
@@ -105,6 +104,7 @@ class LoginManager
             return;
         }
 
+        $this->isLoggedIn = true;
         $this->sessionManager->extendSession();
     }
 
diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php
index 24e25528..b8b8ab8d 100644
--- a/application/security/SessionManager.php
+++ b/application/security/SessionManager.php
@@ -169,6 +169,9 @@ class SessionManager
      */
     public function hasSessionExpired()
     {
+        if (empty($this->session['expires_on'])) {
+            return true;
+        }
         if (time() >= $this->session['expires_on']) {
             return true;
         }
@@ -188,7 +191,7 @@ class SessionManager
         if ($this->conf->get('security.session_protection_disabled') === true) {
             return false;
         }
-        if ($this->session['ip'] == $clientIpId) {
+        if (isset($this->session['ip']) && $this->session['ip'] === $clientIpId) {
             return false;
         }
         return true;
diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php
index fad09992..f26cd1eb 100644
--- a/tests/security/LoginManagerTest.php
+++ b/tests/security/LoginManagerTest.php
@@ -84,10 +84,7 @@ class LoginManagerTest extends TestCase
         $this->globals = &$GLOBALS;
         unset($this->globals['IPBANS']);
 
-        $this->session = [
-            'expires_on' => time() + 100,
-            'ip' => $this->clientIpAddress,
-        ];
+        $this->session = [];
 
         $this->sessionManager = new SessionManager($this->session, $this->configManager);
         $this->loginManager = new LoginManager($this->globals, $this->configManager, $this->sessionManager);
@@ -281,12 +278,18 @@ class LoginManagerTest extends TestCase
      */
     public function testCheckLoginStateStaySignedInWithInvalidToken()
     {
+        // simulate a previous login
+        $this->session = [
+            'ip' => $this->clientIpAddress,
+            'expires_on' => time() + 100,
+        ];
         $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
         $this->cookie[LoginManager::$STAY_SIGNED_IN_COOKIE] = 'nope';
 
         $this->loginManager->checkLoginState($this->cookie, $this->clientIpAddress);
 
-        $this->assertFalse($this->loginManager->isLoggedIn());
+        $this->assertTrue($this->loginManager->isLoggedIn());
+        $this->assertTrue(empty($this->session['username']));
     }
 
     /**
@@ -300,6 +303,8 @@ class LoginManagerTest extends TestCase
         $this->loginManager->checkLoginState($this->cookie, $this->clientIpAddress);
 
         $this->assertTrue($this->loginManager->isLoggedIn());
+        $this->assertEquals($this->login, $this->session['username']);
+        $this->assertEquals($this->clientIpAddress, $this->session['ip']);
     }
 
     /**
-- 
2.41.0