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.
Completed
Push — master ( 17ed34...f4bd34 )
by Omar El
03:18
created

Session::set()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 3
Code Lines 2

Duplication

Lines 0
Ratio 0 %
Metric Value
dl 0
loc 3
rs 10
cc 1
eloc 2
nc 1
nop 2
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
        $session_id = session_id();
28
        if (empty($session_id)) {
29
            session_start();
30
        }
31
    }
32
33
    /**
34
     * Checks if session data exists and valid or not.
35
     *
36
     * @access public
37
     * @static static method
38
     * @param  string $ip
39
     * @param  string $userAgent
40
     * @return boolean
41
     *
42
     */
43
    public static function isSessionValid($ip, $userAgent){
44
45
        $isLoggedIn  = self::getIsLoggedIn();
46
        $userId      = self::getUserId();
47
        $userRole    = self::getUserRole();
48
49
        // 1. check if there is any data in session
50
        if(empty($isLoggedIn) || empty($userId) || empty($userRole)){
51
            return false;
52
        }
53
54
        /*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...
55
            self::remove();
56
            return false;
57
        }*/
58
59
        // 2. then check ip address and user agent
60
        if(!self::validateIPAddress($ip) || !self::validateUserAgent($userAgent)) {
61
            Logger::log("SESSION", "current session is invalid", __FILE__, __LINE__);
62
            self::remove();
63
            return false;
64
        }
65
66
        // 3. check if session is expired
67
        if(!self::validateSessionExpiry()){
68
            self::remove();
69
            return false;
70
        }
71
72
        return true;
73
    }
74
75
    /**
76
     * Get IsLoggedIn value(boolean)
77
     *
78
     * @access public
79
     * @static static method
80
     * @return boolean
81
     *
82
     */
83
    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...
84
        return empty($_SESSION["is_logged_in"]) || !is_bool($_SESSION["is_logged_in"]) ? false : $_SESSION["is_logged_in"];
85
    }
86
87
    /**
88
     * Get User ID.
89
     *
90
     * @access public
91
     * @static static method
92
     * @return string|null
93
     *
94
     */
95
    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...
96
        return empty($_SESSION["user_id"]) ? null : (int)$_SESSION["user_id"];
97
    }
98
99
    /**
100
     * Get User Role
101
     *
102
     * @access public
103
     * @static static method
104
     * @return string|null
105
     *
106
     */
107
    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...
108
        return empty($_SESSION["role"]) ? null : $_SESSION["role"];
109
    }
110
111
    /**
112
     * Get CSRF Token
113
     *
114
     * @access public
115
     * @static static method
116
     * @return string|null
117
     *
118
     */
119
    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...
120
        return empty($_SESSION["csrf_token"]) ? null : $_SESSION["csrf_token"];
121
    }
122
123
    /**
124
     * Get CSRF Token generated time
125
     *
126
     * @access public
127
     * @static static method
128
     * @return string|null
129
     *
130
     */
131
    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...
132
        return empty($_SESSION["csrf_token_time"]) ? null : $_SESSION["csrf_token_time"];
133
    }
134
135
    /**
136
     * set session key and value
137
     *
138
     * @access public
139
     * @static static method
140
     * @param $key
141
     * @param $value
142
     *
143
     */
144
    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...
145
        $_SESSION[$key] = $value;
146
    }
147
148
    /**
149
     * get session value by $key
150
     *
151
     * @access public
152
     * @static static method
153
     * @param  $key
154
     * @return mixed
155
     *
156
     */
157
    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...
158
        return array_key_exists($key, $_SESSION)? $_SESSION[$key]: null;
159
    }
160
161
    /**
162
     * get session value by $key and destroy it
163
     *
164
     * @access public
165
     * @static static method
166
     * @param  $key
167
     * @return mixed
168
     *
169
     */
170
    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...
171
172
        if(array_key_exists($key, $_SESSION)){
173
174
            $value = $_SESSION[$key];
175
            $_SESSION[$key] = null;
176
            unset($_SESSION[$key]);
177
178
            return $value;
179
        }
180
        
181
        return null;
182
    }
183
184
    /**
185
     * matches current IP Address with the one stored in the session
186
     *
187
     * @access public
188
     * @static static method
189
     * @param  string $ip
190
     * @return bool
191
     *
192
     */
193
    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...
194
195
        if(!isset($_SESSION['ip']) || !isset($ip)) {
196
            return false;
197
        }
198
199
        return $_SESSION['ip'] === $ip;
200
    }
201
202
    /**
203
     * matches current user agent with the one stored in the session
204
     *
205
     * @access public
206
     * @static static method
207
     * @param  string $userAgent
208
     * @return bool
209
     *
210
     */
211
    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...
212
213
        if(!isset($_SESSION['user_agent']) || !isset($userAgent)) {
214
            return false;
215
        }
216
217
        return $_SESSION['user_agent'] === $userAgent;
218
    }
219
220
    /**
221
     * checks if session has been expired
222
     *
223
     * @access public
224
     * @static static method
225
     * @return bool
226
     *
227
     */
228
    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...
229
230
        $max_time = 60 * 60 * 24; // 1 day
231
232
        if(!isset($_SESSION['generated_time'])) {
233
            return false;
234
        }
235
236
        return ($_SESSION['generated_time'] + $max_time) > time();
237
    }
238
239
    /**
240
     * checks for session concurrency
241
     *
242
     * This is done as the following:
243
     * UserA logs in with his session id('123') and it will be stored in the database.
244
     * Then, UserB logs in also using the same email and password of UserA from another PC,
245
     * and also store the session id('456') in the database
246
     *
247
     * Now, Whenever UserA performs any action,
248
     * You then check the session_id() against the last one stored in the database('456'),
249
     * If they don't match then log both of them out.
250
     *
251
     * @access public
252
     * @static static method
253
     * @return bool
254
     * @see Session::updateSessionId()
255
     * @see http://stackoverflow.com/questions/6126285/php-stop-concurrent-user-logins
256
     */
257
    public static function isConcurrentSessionExists(){
258
259
        $session_id = session_id();
260
        $userId  = self::getUserId();
261
262
        if(isset($userId) && isset($session_id)){
263
264
            $database = Database::openConnection();
265
            $database->prepare("SELECT session_id FROM users WHERE id = :id LIMIT 1");
266
267
            $database->bindValue(":id", $userId);
268
            $database->execute();
269
            $result = $database->fetchAssociative();
270
            $userSessionId = !empty($result)? $result['session_id']: null;
271
272
            return $session_id !== $userSessionId;
273
        }
274
275
        return false;
276
    }
277
278
    /**
279
     * get CSRF token and generate a new one if expired
280
     *
281
     * @access public
282
     * @static static method
283
     * @return string
284
     *
285
     */
286
    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...
287
288
        $max_time = 60 * 60 * 24; // 1 day
289
        $stored_time = self::getCsrfTokenTime();
290
        $csrf_token  = self::getCsrfToken();
291
292
        if($max_time + $stored_time <= time() || empty($csrf_token)){
293
            $token = md5(uniqid(rand(), true));
294
            $_SESSION["csrf_token"] = $token;
295
            $_SESSION["csrf_token_time"] = time();
296
        }
297
298
        return self::getCsrfToken();
299
    }
300
301
    /**
302
     * reset session id, delete session file on server, and re-assign the values.
303
     *
304
     * @access public
305
     * @static static method
306
     * @param  array  $data
307
     * @return string
308
     *
309
     */
310
    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...
311
312
        // remove old and regenerate session ID.
313
        session_regenerate_id(true);
314
        $_SESSION = array();
315
316
        $_SESSION["is_logged_in"] = true;
317
        $_SESSION["user_id"]      = (int)$data["user_id"];
318
        $_SESSION["role"]         = $data["role"];
319
320
        // save these values in the session,
321
        // they are needed to avoid session hijacking and fixation
322
        $_SESSION['ip']             = $data["ip"];
323
        $_SESSION['user_agent']     = $data["user_agent"];
324
        $_SESSION['generated_time'] = time();
325
326
        // update session id in database
327
        self::updateSessionId($data["user_id"], session_id());
328
329
        // set session cookie setting manually,
330
        // Why? because you need to explicitly set session expiry, path, domain, secure, and HTTP.
331
        // @see https://www.owasp.org/index.php/PHP_Security_Cheat_Sheet#Cookies
332
        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'));
333
    }
334
335
    /**
336
     * update session id in database
337
     *
338
     * @access public
339
     * @static static method
340
     * @param  string $userId
341
     * @param  string $sessionId
342
     * @return string
343
     *
344
     */
345 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...
346
347
        $database = Database::openConnection();
348
        $database->prepare("UPDATE users SET session_id = :session_id WHERE id = :id");
349
350
        $database->bindValue(":session_id", $sessionId);
351
        $database->bindValue(":id", $userId);
352
        $database->execute();
353
    }
354
355
    /**
356
     * Remove the session
357
     * Either by regenerating the session id & file, and clear the session data
358
     * Or, delete session completely from the browser cookies and destroy it's file on the server
359
     *
360
     * @access public
361
     * @static static method
362
     * @param  bool   $keep   True to keep session in browser, and just regenerate it's id, and clear data.
363
     *                        False to destroy session cookie in browser completely and it's file on server
364
     */
365
    public static function remove($keep = false){
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...
366
367
        // update session in database
368
        $userId = self::getUserId();
369
        if(!empty($userId)){
370
            self::updateSessionId(self::getUserId());
371
        }
372
373
        if($keep){
374
375
            // regenerate it's id in browser, and file
376
            // also clear any data stored in session
377
            session_regenerate_id(true);
378
            $_SESSION = array();
379
380
        } else {
381
382
            // clear session data
383
            $_SESSION = array();
384
385
            // remove session cookie
386
            if (ini_get("session.use_cookies")) {
387
                $params = session_get_cookie_params();
388
                setcookie(session_name(), '', time() - 42000,
389
                    $params["path"], $params["domain"],
390
                    $params["secure"], $params["httponly"]
391
                );
392
            }
393
394
            // destroy session file on server
395
            session_destroy();
396
        }
397
    }
398
399
}
400