GitHub Access Token became invalid

It seems like the GitHub access token used for retrieving details about this repository from GitHub became invalid. This might prevent certain types of inspections from being run (in particular, everything related to pull requests).
Please ask an admin of your repository to re-new the access token on this website.

Session::getUserId()   A
last analyzed

Complexity

Conditions 2
Paths 2

Size

Total Lines 3
Code Lines 2

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 3
rs 10
c 0
b 0
f 0
cc 2
eloc 2
nc 2
nop 0
1
<?php
2
3
/**
4
 * Session Class
5
 *
6
 * @license    http://opensource.org/licenses/MIT The MIT License (MIT)
7
 * @author     Omar El Gabry <[email protected]>
8
 */
9
10
class Session{
0 ignored issues
show
Coding Style Compatibility introduced by
PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions.

You can fix this by adding a namespace to your class:

namespace YourVendor;

class YourClass { }

When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.

Loading history...
11
12
    /**
13
     * constructor for Session Object.
14
     *
15
     * @access private
16
     */
17
    private function __construct() {}
18
19
    /**
20
     * Starts the session if not started yet.
21
     *
22
     * @access public
23
     *
24
     */
25
    public static function init(){
26
27
        if (session_status() == PHP_SESSION_NONE) {     // if (session_id() == '')
0 ignored issues
show
Unused Code Comprehensibility introduced by
50% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
28
            session_start();
29
        }
30
    }
31
32
    /**
33
     * Checks if session data exists and valid or not.
34
     *
35
     * @access public
36
     * @static static method
37
     * @param  string $ip
38
     * @param  string $userAgent
39
     * @return boolean
40
     *
41
     */
42
    public static function isSessionValid($ip, $userAgent){
43
44
        $isLoggedIn  = self::getIsLoggedIn();
45
        $userId      = self::getUserId();
46
        $userRole    = self::getUserRole();
47
48
        // 1. check if there is any data in session
49
        if(empty($isLoggedIn) || empty($userId) || empty($userRole)){
50
            return false;
51
        }
52
53
        /*if(!self::isConcurrentSessionExists()){
0 ignored issues
show
Unused Code Comprehensibility introduced by
75% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
54
            self::remove();
55
            return false;
56
        }*/
57
58
        // 2. then check ip address and user agent
59
        if(!self::validateIPAddress($ip) || !self::validateUserAgent($userAgent)) {
60
            Logger::log("SESSION", "current session is invalid", __FILE__, __LINE__);
61
            self::remove();
62
            return false;
63
        }
64
65
        // 3. check if session is expired
66
        if(!self::validateSessionExpiry()){
67
            self::remove();
68
            return false;
69
        }
70
71
        return true;
72
    }
73
74
    /**
75
     * Get IsLoggedIn value(boolean)
76
     *
77
     * @access public
78
     * @static static method
79
     * @return boolean
80
     *
81
     */
82
    public static function getIsLoggedIn(){
0 ignored issues
show
Coding Style introduced by
getIsLoggedIn 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...
83
        return empty($_SESSION["is_logged_in"]) || !is_bool($_SESSION["is_logged_in"]) ? false : $_SESSION["is_logged_in"];
84
    }
85
86
    /**
87
     * Get User ID.
88
     *
89
     * @access public
90
     * @static static method
91
     * @return string|null
92
     *
93
     */
94
    public static function getUserId(){
0 ignored issues
show
Coding Style introduced by
getUserId 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...
95
        return empty($_SESSION["user_id"]) ? null : (int)$_SESSION["user_id"];
96
    }
97
98
    /**
99
     * Get User Role
100
     *
101
     * @access public
102
     * @static static method
103
     * @return string|null
104
     *
105
     */
106
    public static function getUserRole(){
0 ignored issues
show
Coding Style introduced by
getUserRole 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...
107
        return empty($_SESSION["role"]) ? null : $_SESSION["role"];
108
    }
109
110
    /**
111
     * Get CSRF Token
112
     *
113
     * @access public
114
     * @static static method
115
     * @return string|null
116
     *
117
     */
118
    public static function getCsrfToken(){
0 ignored issues
show
Coding Style introduced by
getCsrfToken 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...
119
        return empty($_SESSION["csrf_token"]) ? null : $_SESSION["csrf_token"];
120
    }
121
122
    /**
123
     * Get CSRF Token generated time
124
     *
125
     * @access public
126
     * @static static method
127
     * @return string|null
128
     *
129
     */
130
    public static function getCsrfTokenTime(){
0 ignored issues
show
Coding Style introduced by
getCsrfTokenTime 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...
131
        return empty($_SESSION["csrf_token_time"]) ? null : $_SESSION["csrf_token_time"];
132
    }
133
134
    /**
135
     * set session key and value
136
     *
137
     * @access public
138
     * @static static method
139
     * @param $key
140
     * @param $value
141
     *
142
     */
143
    public static function set($key, $value){
0 ignored issues
show
Coding Style introduced by
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...
144
        $_SESSION[$key] = $value;
145
    }
146
147
    /**
148
     * get session value by $key
149
     *
150
     * @access public
151
     * @static static method
152
     * @param  $key
153
     * @return mixed
154
     *
155
     */
156
    public static function get($key){
0 ignored issues
show
Coding Style introduced by
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...
157
        return array_key_exists($key, $_SESSION)? $_SESSION[$key]: null;
158
    }
159
160
    /**
161
     * get session value by $key and destroy it
162
     *
163
     * @access public
164
     * @static static method
165
     * @param  $key
166
     * @return mixed
167
     *
168
     */
169
    public static function getAndDestroy($key){
0 ignored issues
show
Coding Style introduced by
getAndDestroy 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...
170
171
        if(array_key_exists($key, $_SESSION)){
172
173
            $value = $_SESSION[$key];
174
            $_SESSION[$key] = null;
175
            unset($_SESSION[$key]);
176
177
            return $value;
178
        }
179
        
180
        return null;
181
    }
182
183
    /**
184
     * matches current IP Address with the one stored in the session
185
     *
186
     * @access public
187
     * @static static method
188
     * @param  string $ip
189
     * @return bool
190
     *
191
     */
192
    private static function validateIPAddress($ip){
0 ignored issues
show
Coding Style introduced by
validateIPAddress 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...
193
194
        if(!isset($_SESSION['ip']) || !isset($ip)) {
195
            return false;
196
        }
197
198
        return $_SESSION['ip'] === $ip;
199
    }
200
201
    /**
202
     * matches current user agent with the one stored in the session
203
     *
204
     * @access public
205
     * @static static method
206
     * @param  string $userAgent
207
     * @return bool
208
     *
209
     */
210
    private static function validateUserAgent($userAgent){
0 ignored issues
show
Coding Style introduced by
validateUserAgent 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...
211
212
        if(!isset($_SESSION['user_agent']) || !isset($userAgent)) {
213
            return false;
214
        }
215
216
        return $_SESSION['user_agent'] === $userAgent;
217
    }
218
219
    /**
220
     * checks if session has been expired
221
     *
222
     * @access public
223
     * @static static method
224
     * @return bool
225
     *
226
     */
227
    private static function validateSessionExpiry(){
0 ignored issues
show
Coding Style introduced by
validateSessionExpiry 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...
228
229
        $max_time = 60 * 60 * 24; // 1 day
230
231
        if(!isset($_SESSION['generated_time'])) {
232
            return false;
233
        }
234
235
        return ($_SESSION['generated_time'] + $max_time) > time();
236
    }
237
238
    /**
239
     * checks for session concurrency
240
     *
241
     * This is done as the following:
242
     * UserA logs in with his session id('123') and it will be stored in the database.
243
     * Then, UserB logs in also using the same email and password of UserA from another PC,
244
     * and also store the session id('456') in the database
245
     *
246
     * Now, Whenever UserA performs any action,
247
     * You then check the session_id() against the last one stored in the database('456'),
248
     * If they don't match then log both of them out.
249
     *
250
     * @access public
251
     * @static static method
252
     * @return bool
253
     * @see Session::updateSessionId()
254
     * @see http://stackoverflow.com/questions/6126285/php-stop-concurrent-user-logins
255
     */
256
    public static function isConcurrentSessionExists(){
257
258
        $session_id = session_id();
259
        $userId  = self::getUserId();
260
261
        if(isset($userId) && isset($session_id)){
262
263
            $database = Database::openConnection();
264
            $database->prepare("SELECT session_id FROM users WHERE id = :id LIMIT 1");
265
266
            $database->bindValue(":id", $userId);
267
            $database->execute();
268
            $result = $database->fetchAssociative();
269
            $userSessionId = !empty($result)? $result['session_id']: null;
270
271
            return $session_id !== $userSessionId;
272
        }
273
274
        return false;
275
    }
276
277
    /**
278
     * get CSRF token and generate a new one if expired
279
     *
280
     * @access public
281
     * @static static method
282
     * @return string
283
     *
284
     */
285
    public static function generateCsrfToken(){
0 ignored issues
show
Coding Style introduced by
generateCsrfToken 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...
286
287
        $max_time = 60 * 60 * 24; // 1 day
288
        $stored_time = self::getCsrfTokenTime();
289
        $csrf_token  = self::getCsrfToken();
290
291
        if($max_time + $stored_time <= time() || empty($csrf_token)){
292
            $token = md5(uniqid(rand(), true));
293
            $_SESSION["csrf_token"] = $token;
294
            $_SESSION["csrf_token_time"] = time();
295
        }
296
297
        return self::getCsrfToken();
298
    }
299
300
    /**
301
     * reset session id, delete session file on server, and re-assign the values.
302
     *
303
     * @access public
304
     * @static static method
305
     * @param  array  $data
306
     * @return string
307
     *
308
     */
309
    public static function reset($data){
0 ignored issues
show
Coding Style introduced by
reset 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...
310
311
        // remove old and regenerate session ID.
312
        session_regenerate_id(true);
313
        $_SESSION = array();
314
315
        $_SESSION["is_logged_in"] = true;
316
        $_SESSION["user_id"]      = (int)$data["user_id"];
317
        $_SESSION["role"]         = $data["role"];
318
319
        // save these values in the session,
320
        // they are needed to avoid session hijacking and fixation
321
        $_SESSION['ip']             = $data["ip"];
322
        $_SESSION['user_agent']     = $data["user_agent"];
323
        $_SESSION['generated_time'] = time();
324
325
        // update session id in database
326
        self::updateSessionId($data["user_id"], session_id());
327
328
        // set session cookie setting manually,
329
        // Why? because you need to explicitly set session expiry, path, domain, secure, and HTTP.
330
        // @see https://www.owasp.org/index.php/PHP_Security_Cheat_Sheet#Cookies
331
        setcookie(session_name(), session_id(), time() + Config::get('SESSION_COOKIE_EXPIRY') /*a week*/, Config::get('COOKIE_PATH'), Config::get('COOKIE_DOMAIN'), Config::get('COOKIE_SECURE'), Config::get('COOKIE_HTTP'));
332
    }
333
334
    /**
335
     * update session id in database
336
     *
337
     * @access public
338
     * @static static method
339
     * @param  string $userId
340
     * @param  string $sessionId
341
     * @return string
342
     *
343
     */
344 View Code Duplication
    private static function updateSessionId($userId, $sessionId = null){
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
345
346
        $database = Database::openConnection();
347
        $database->prepare("UPDATE users SET session_id = :session_id WHERE id = :id");
348
349
        $database->bindValue(":session_id", $sessionId);
350
        $database->bindValue(":id", $userId);
351
        $database->execute();
352
    }
353
354
    /**
355
     * Remove the session
356
     * Delete session completely from the browser cookies and destroy it's file on the server
357
     *
358
     * @access public
359
     * @static static method
360
     */
361
    public static function remove(){
0 ignored issues
show
Coding Style introduced by
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...
362
363
        // update session in database
364
        $userId = self::getUserId();
365
        if(!empty($userId)){
366
            self::updateSessionId(self::getUserId());
367
        }
368
369
        // clear session data
370
        $_SESSION = array();
371
372
        // remove session cookie
373
        if (ini_get("session.use_cookies")) {
374
            $params = session_get_cookie_params();
375
            setcookie(session_name(), '', time() - 42000,
376
                $params["path"], $params["domain"],
377
                $params["secure"], $params["httponly"]
378
            );
379
        }
380
381
        // destroy session file on server(if not already)
382
        if(session_status() === PHP_SESSION_ACTIVE){
383
            session_destroy();
384
        }
385
    }
386
387
}
388