From ebf615173824a46de82fa97a165bcfd883db15ce Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Thu, 10 May 2018 13:07:51 +0200
Subject: [PATCH] SessionManager: remove unused UID token
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

There already are dedicated tokens for:
- CSRF protection
- user stay-signed-in feature, via cookie

This token was most likely intended as a randomly generated,
server-side, secret key to be used when generating hashes.

See http://sebsauvage.net/wiki/doku.php?id=php:session [FR]

Relevant section:

  Une clé secrète unique aléatoire est générée côté serveur (et jamais
  envoyée). Elle peut servir pour signer les formulaires (HMAC) ou
  générer des token de formulaires (protection contre XSRF).
  Voir $_SESSION['uid'].

Translation:

  A unique, server-side secret key is randomly generated (and never
  transmitted). It can be used to sign forms (HMAC) or generate form
  tokens (protection against XSRF).
  See $_SESSION['uid']

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/security/SessionManager.php |  6 ------
 tests/security/SessionManagerTest.php   | 13 -------------
 2 files changed, 19 deletions(-)

diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php
index 58973130..24e25528 100644
--- a/application/security/SessionManager.php
+++ b/application/security/SessionManager.php
@@ -113,8 +113,6 @@ class SessionManager
      */
     public function storeLoginInfo($clientIpId)
     {
-        // Generate unique random number (different than phpsessionid)
-        $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
         $this->session['ip'] = $clientIpId;
         $this->session['username'] = $this->conf->get('credentials.login');
         $this->extendTimeValidityBy(self::$SHORT_TIMEOUT);
@@ -154,7 +152,6 @@ class SessionManager
     public function logout()
     {
         if (isset($this->session)) {
-            unset($this->session['uid']);
             unset($this->session['ip']);
             unset($this->session['expires_on']);
             unset($this->session['username']);
@@ -172,9 +169,6 @@ class SessionManager
      */
     public function hasSessionExpired()
     {
-        if (empty($this->session['uid'])) {
-            return true;
-        }
         if (time() >= $this->session['expires_on']) {
             return true;
         }
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php
index e1c72707..ae10ffa6 100644
--- a/tests/security/SessionManagerTest.php
+++ b/tests/security/SessionManagerTest.php
@@ -164,7 +164,6 @@ class SessionManagerTest extends TestCase
     {
         $this->sessionManager->storeLoginInfo('ip_id');
 
-        $this->assertTrue(isset($this->session['uid']));
         $this->assertGreaterThan(time(), $this->session['expires_on']);
         $this->assertEquals('ip_id', $this->session['ip']);
         $this->assertEquals('johndoe', $this->session['username']);
@@ -209,7 +208,6 @@ class SessionManagerTest extends TestCase
     public function testLogout()
     {
         $this->session = [
-            'uid' => 'some-uid',
             'ip' => 'ip_id',
             'expires_on' => time() + 1000,
             'username' => 'johndoe',
@@ -218,7 +216,6 @@ class SessionManagerTest extends TestCase
         ];
         $this->sessionManager->logout();
 
-        $this->assertFalse(isset($this->session['uid']));
         $this->assertFalse(isset($this->session['ip']));
         $this->assertFalse(isset($this->session['expires_on']));
         $this->assertFalse(isset($this->session['username']));
@@ -226,20 +223,11 @@ class SessionManagerTest extends TestCase
         $this->assertFalse(isset($this->session['untaggedonly']));
     }
 
-    /**
-     * The session is considered as expired because the UID is missing
-     */
-    public function testHasExpiredNoUid()
-    {
-        $this->assertTrue($this->sessionManager->hasSessionExpired());
-    }
-
     /**
      * The session is active and expiration time has been reached
      */
     public function testHasExpiredTimeElapsed()
     {
-        $this->session['uid'] = 'some-uid';
         $this->session['expires_on'] = time() - 10;
 
         $this->assertTrue($this->sessionManager->hasSessionExpired());
@@ -250,7 +238,6 @@ class SessionManagerTest extends TestCase
      */
     public function testHasNotExpired()
     {
-        $this->session['uid'] = 'some-uid';
         $this->session['expires_on'] = time() + 1000;
 
         $this->assertFalse($this->sessionManager->hasSessionExpired());
-- 
2.41.0