Manager - Code Metrics - Inspection of "Exception during session_regenerate_id" - XOOPS/XoopsCore - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#532)

by Richard

created 2017-03-11 21:57 UTC

Manager A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	267
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	7

Test Coverage

Coverage

7.79%

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
dl	0
loc	267
ccs	6
cts	77
cp	0.0779
rs	10
c	1
b	0
f	0
wmc	25
lcom	1
cbo	7

13 Methods

Rating	Name	Size	Complexity
A	__construct()	7	1
B	sessionStart()	63	7
A	clearSession()	6	1
A	regenerateSession()	4	1
A	validateSession()	14	4
A	user()	4	1
A	sessionShutdown()	5	1
A	get()	4	2
A	has()	4	1
A	remove()	7	2
A	clear()	6	1
A	set()	5	1
B	expireSession()	26	2

<?php
/*
 You may not change or alter any portion of this comment or credits
 of supporting developers from this source code or any supporting source code
 which is considered copyrighted (c) material of the original comment or credit authors.

 This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
*/

namespace Xoops\Core\Session;

use Xoops\Core\AttributeInterface;
use Xoops\Core\HttpRequest;

/**
 * Session management
 *
 * Credits due to Robert Hafner's article "How to Create Bulletproof Sessions"
 * see: http://blog.teamtreehouse.com/how-to-create-bulletproof-sessions
 *
 * @category  Xoops\Core\Session
 * @package   Manager
 * @author    Richard Griffith <[email protected]>
 * @copyright 2015 XOOPS Project (http://xoops.org)
 * @license   GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
 * @link      http://xoops.org
 */
class Manager implements AttributeInterface
{
    /**
     * @var \Xoops
     */
    protected $xoops = null;

    /**
     * @var \Xoops\Core\HttpRequest
     */
    protected $httpRequest = null;

    /**
     * @var Fingerprint fingerprint object
     */
    protected $fingerprint = null;

    /**
     * @var SessionUser session user object
     */
    protected $sessionUser = null;

    /**
     * establish access to other classes we will use
     */
    public function __construct()
    {
        $this->xoops = \Xoops::getInstance();
        $this->httpRequest = HttpRequest::getInstance();
        $this->sessionUser = new SessionUser($this);
        $this->fingerprint = new Fingerprint;
    }

    /**
     * Configure and start the session
     *
     * @return void
     */
    public function sessionStart()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        /**
         * Revisit this once basics are working
         *
         * grab session_id from https login form
         *
         *  if ($xoops->getConfig('use_ssl')
         *      && isset($_POST[$xoops->getConfig('sslpost_name')])
         *      && $_POST[$xoops->getConfig('sslpost_name')] != ''
         *  ) {
         *      session_id($_POST[$xoops->getConfig('sslpost_name')]);
         *  } else { set session_name...}
         */

        $name = $this->xoops->getConfig('session_name');
        $name = (empty($name)) ? 'xoops_session' : $name;
        $expire = (int)($this->xoops->getConfig('session_expire'));
        $expire = ($expire>0) ? $expire : 300;

        $path = \XoopsBaseConfig::get('cookie-path');
        $domain = \XoopsBaseConfig::get('cookie-domain');
        $secure = $this->httpRequest->is('ssl');
        session_name($name);
        session_cache_expire($expire);

        session_set_cookie_params(0, $path, $domain, $secure, true);

        $sessionHandler = new Handler;
        session_set_save_handler($sessionHandler);

        //session_register_shutdown();
        register_shutdown_function(array($this, 'sessionShutdown'));

        session_start();

        // if session is empty, make sure it isn't using a passed in id
        if (empty($_SESSION)) {
            $this->regenerateSession();
        }

        // Make sure the session hasn't expired, and destroy it if it has
        if (!$this->validateSession()) {
            $this->clearSession();
            return;
        }

        // Check to see if the session shows sign of hijacking attempt
        if (!$this->fingerprint->checkSessionPrint($this)) {
            $this->regenerateSession(); // session data already cleared, just needs new id
            return;
        }

        // establish valid user data in session, possibly clearing or adding from
        // RememberMe mechanism as needed
        $this->sessionUser->establish();

        // Give a 5% chance of the session id changing on any authenticated request
        //if ($this->has('xoopsUserId') && (rand(1, 100) <= 5)) {
        if ((rand(1, 100) <= 5)) {
            $this->expireSession();
        }
    }

    /**
     * Clear the current session and reset fingerprint
     *
     * @return void
     */
    public function clearSession()
    {
        $this->clear();
        $this->fingerprint->checkSessionPrint($this);
        $this->regenerateSession();
    }

    /**
     * Expire the current session and replace with a fresh one.
     *
     * @return void
     */
    public function expireSession()
    {
        // If this session is obsolete it means there already is a new id
        if ($this->has('SESSION_MANAGER_OBSOLETE')) {
            return;
        }

        // Set current session to expire in 10 seconds
        $this->set('SESSION_MANAGER_OBSOLETE', true);
        $this->set('SESSION_MANAGER_EXPIRES', time() + 10);

        // Grab current session ID and close it
        //$sessionId = session_id();
        //session_write_close();

        // reopen the old session
        //session_id($sessionId);
        //session_start();

        // Create new session without destroying the old one
        session_regenerate_id(false);

        // Now we unset the obsolete and expiration values since we ant to keep this one
        $this->remove('SESSION_MANAGER_OBSOLETE');
        $this->remove('SESSION_MANAGER_EXPIRES');
    }

    /**
     * Generate a new id and delete the old session.
     *
     * This should be called whenever permission levels for a user change.
     *
     * @return void
     */
    public function regenerateSession()
    {
        session_regenerate_id(true);
    }

    /**
     * Validate that the session has not expired.
     *
     * @return boolean true is session is valid and not expired, otherwise false
     */
    protected function validateSession()
    {
        // invalid to have obsolete and not expires
        if ($this->has('SESSION_MANAGER_OBSOLETE') && !$this->has('SESSION_MANAGER_EXPIRES')) {
            return false;
        }

        // if we don't have the expires key, use a future value for test
        if ($this->get('SESSION_MANAGER_EXPIRES', time()+10) < time()) {
            return false;
        }

        return true;
    }

    /**
     * Get the user object used by this session.
     *
     * @return SessionUser
     */
    public function user()
    {
        return $this->sessionUser;
    }

    /**
     * shutdown function
     */
    public function sessionShutdown()
    {
        \Xoops::getInstance()->events()->triggerEvent('core.session.shutdown');
        session_write_close();
    }

    // access session variables as attribute object

    /**
     * Retrieve a session variable value.
     *
     * @param string $name    Name of an session variable
     * @param mixed  $default A default value returned if the requested
     *                        named session variable is not set.
     *
     * @return  mixed  The value of the session variable, or $default if not set.
     */
    public function get($name, $default = null)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        return (isset($_SESSION[$name])) ? $_SESSION[$name] : $default;
    }

    /**
     * Set an attribute value.
     *
     * @param string $name  Name of the attribute option
     * @param mixed  $value Value of the attribute option
     *
     * @return $this
     */
    public function set($name, $value)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $_SESSION[$name] = $value;
        return $this;
    }

    /**
     * Determine if an attribute exists.
     *
     * @param string $name An attribute name.
     *
     * @return boolean TRUE if the given attribute exists, otherwise FALSE.
     */
    public function has($name)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        return isset($_SESSION[$name]);
    }

    /**
     * Remove an attribute.
     *
     * @param string $name An attribute name.
     *
     * @return mixed An attribute value, if the named attribute existed and
     *               has been removed, otherwise NULL.
     */
    public function remove($name)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $value = (isset($_SESSION[$name])) ? $_SESSION[$name] : null;
        unset($_SESSION[$name]);

        return $value;
    }

    /**
     * Remove all attributes.
     *
     * @return array old values
     */
    public function clear()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $oldValues = $_SESSION;
        $_SESSION = array();
        return $oldValues;
    }
}


1		<?php
2		/*
3		You may not change or alter any portion of this comment or credits
4		of supporting developers from this source code or any supporting source code
5		which is considered copyrighted (c) material of the original comment or credit authors.
6
7		This program is distributed in the hope that it will be useful,
8		but WITHOUT ANY WARRANTY; without even the implied warranty of
9		MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
10		*/
11
12		namespace Xoops\Core\Session;
13
14		use Xoops\Core\AttributeInterface;
15		use Xoops\Core\HttpRequest;
16
17		/**
18		* Session management
19		*
20		* Credits due to Robert Hafner's article "How to Create Bulletproof Sessions"
21		* see: http://blog.teamtreehouse.com/how-to-create-bulletproof-sessions
22		*
23		* @category Xoops\Core\Session
24		* @package Manager
25		* @author Richard Griffith <[email protected]>
26		* @copyright 2015 XOOPS Project (http://xoops.org)
27		* @license GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
28		* @link http://xoops.org
29		*/
30		class Manager implements AttributeInterface
31		{
32		/**
33		* @var \Xoops
34		*/
35		protected $xoops = null;
36
37		/**
38		* @var \Xoops\Core\HttpRequest
39		*/
40		protected $httpRequest = null;
41
42		/**
43		* @var Fingerprint fingerprint object
44		*/
45		protected $fingerprint = null;
46
47		/**
48		* @var SessionUser session user object
49		*/
50		protected $sessionUser = null;
51
52		/**
53		* establish access to other classes we will use
54		*/
55	2	public function __construct()
56		{
57	2	$this->xoops = \Xoops::getInstance();
58	2	$this->httpRequest = HttpRequest::getInstance();
59	2	$this->sessionUser = new SessionUser($this);
60	2	$this->fingerprint = new Fingerprint;
61	2	}
62
63		/**
64		* Configure and start the session
65		*
66		* @return void
67		*/
68		public function sessionStart()
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `sessionStart` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
69		{
70		/**
71		* Revisit this once basics are working
72		*
73		* grab session_id from https login form
74		*
75		* if ($xoops->getConfig('use_ssl')
76		* && isset($_POST[$xoops->getConfig('sslpost_name')])
77		* && $_POST[$xoops->getConfig('sslpost_name')] != ''
78		* ) {
79		* session_id($_POST[$xoops->getConfig('sslpost_name')]);
80		* } else { set session_name...}
81		*/
82
83		$name = $this->xoops->getConfig('session_name');
84		$name = (empty($name)) ? 'xoops_session' : $name;
85		$expire = (int)($this->xoops->getConfig('session_expire'));
86		$expire = ($expire>0) ? $expire : 300;
87
88		$path = \XoopsBaseConfig::get('cookie-path');
89		$domain = \XoopsBaseConfig::get('cookie-domain');
90		$secure = $this->httpRequest->is('ssl');
91		session_name($name);
92		session_cache_expire($expire);
93
94		session_set_cookie_params(0, $path, $domain, $secure, true);
95
96		$sessionHandler = new Handler;
97		session_set_save_handler($sessionHandler);
98
99		//session_register_shutdown();
100		register_shutdown_function(array($this, 'sessionShutdown'));
101
102		session_start();
103
104		// if session is empty, make sure it isn't using a passed in id
105		if (empty($_SESSION)) {
106		$this->regenerateSession();
107		}
108
109		// Make sure the session hasn't expired, and destroy it if it has
110		if (!$this->validateSession()) {
111		$this->clearSession();
112		return;
113		}
114
115		// Check to see if the session shows sign of hijacking attempt
116		if (!$this->fingerprint->checkSessionPrint($this)) {
117		$this->regenerateSession(); // session data already cleared, just needs new id
118		return;
119		}
120
121		// establish valid user data in session, possibly clearing or adding from
122		// RememberMe mechanism as needed
123		$this->sessionUser->establish();
124
125		// Give a 5% chance of the session id changing on any authenticated request
126		//if ($this->has('xoopsUserId') && (rand(1, 100) <= 5)) {
127		if ((rand(1, 100) <= 5)) {
128		$this->expireSession();
129		}
130		}
131
132		/**
133		* Clear the current session and reset fingerprint
134		*
135		* @return void
136		*/
137		public function clearSession()
138		{
139		$this->clear();
140		$this->fingerprint->checkSessionPrint($this);
141		$this->regenerateSession();
142		}
143
144		/**
145		* Expire the current session and replace with a fresh one.
146		*
147		* @return void
148		*/
149		public function expireSession()
150		{
151		// If this session is obsolete it means there already is a new id
152		if ($this->has('SESSION_MANAGER_OBSOLETE')) {
153		return;
154		}
155
156		// Set current session to expire in 10 seconds
157		$this->set('SESSION_MANAGER_OBSOLETE', true);
158		$this->set('SESSION_MANAGER_EXPIRES', time() + 10);
159
160		// Grab current session ID and close it
161		//$sessionId = session_id();
162		//session_write_close();
163
164		// reopen the old session
165		//session_id($sessionId);
166		//session_start();
167
168		// Create new session without destroying the old one
169		session_regenerate_id(false);
170
171		// Now we unset the obsolete and expiration values since we ant to keep this one
172		$this->remove('SESSION_MANAGER_OBSOLETE');
173		$this->remove('SESSION_MANAGER_EXPIRES');
174		}
175
176		/**
177		* Generate a new id and delete the old session.
178		*
179		* This should be called whenever permission levels for a user change.
180		*
181		* @return void
182		*/
183		public function regenerateSession()
184		{
185		session_regenerate_id(true);
186		}
187
188		/**
189		* Validate that the session has not expired.
190		*
191		* @return boolean true is session is valid and not expired, otherwise false
192		*/
193		protected function validateSession()
194		{
195		// invalid to have obsolete and not expires
196		if ($this->has('SESSION_MANAGER_OBSOLETE') && !$this->has('SESSION_MANAGER_EXPIRES')) {
197		return false;
198		}
199
200		// if we don't have the expires key, use a future value for test
201		if ($this->get('SESSION_MANAGER_EXPIRES', time()+10) < time()) {
202		return false;
203		}
204
205		return true;
206		}
207
208		/**
209		* Get the user object used by this session.
210		*
211		* @return SessionUser
212		*/
213		public function user()
214		{
215		return $this->sessionUser;
216		}
217
218		/**
219		* shutdown function
220		*/
221		public function sessionShutdown()
222		{
223		\Xoops::getInstance()->events()->triggerEvent('core.session.shutdown');
224		session_write_close();
225		}
226
227		// access session variables as attribute object
228
229		/**
230		* Retrieve a session variable value.
231		*
232		* @param string $name Name of an session variable
233		* @param mixed $default A default value returned if the requested
234		* named session variable is not set.
235		*
236		* @return mixed The value of the session variable, or $default if not set.
237		*/
238		public function get($name, $default = null)
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `get` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
239		{
240		return (isset($_SESSION[$name])) ? $_SESSION[$name] : $default;
241		}
242
243		/**
244		* Set an attribute value.
245		*
246		* @param string $name Name of the attribute option
247		* @param mixed $value Value of the attribute option
248		*
249		* @return $this
250		*/
251		public function set($name, $value)
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `set` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
252		{
253		$_SESSION[$name] = $value;
254		return $this;
255		}
256
257		/**
258		* Determine if an attribute exists.
259		*
260		* @param string $name An attribute name.
261		*
262		* @return boolean TRUE if the given attribute exists, otherwise FALSE.
263		*/
264		public function has($name)
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `has` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
265		{
266		return isset($_SESSION[$name]);
267		}
268
269		/**
270		* Remove an attribute.
271		*
272		* @param string $name An attribute name.
273		*
274		* @return mixed An attribute value, if the named attribute existed and
275		* has been removed, otherwise NULL.
276		*/
277		public function remove($name)
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `remove` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
278		{
279		$value = (isset($_SESSION[$name])) ? $_SESSION[$name] : null;
280		unset($_SESSION[$name]);
281
282		return $value;
283		}
284
285		/**
286		* Remove all attributes.
287		*
288		* @return array old values
289		*/
290		public function clear()
		0 ignored issues – show Coding Style introduced 2015-12-12 03:50 UTC by Report Bug Copy Issue Report `clear` uses the super-global variable `$_SESSION` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
291		{
292		$oldValues = $_SESSION;
293		$_SESSION = array();
294		return $oldValues;
295		}
296		}
297

XOOPS / XoopsCore

Pull Request — master (#532)

Manager A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

13 Methods

Duplication Side-by-Side

Filter issues like