Session validation improvement

.travis.yml

@@ -30,7 +30,7 @@ before_script:
  - cd tests
 
 # run unit tests, create result file
-script: ../vendor/bin/phpunit --configuration phpunit.xml --coverage-text --coverage-clover=coverage.clover
+script: ../vendor/bin/phpunit --verbose --debug --configuration phpunit.xml --coverage-text --coverage-clover=coverage.clover
 
 # gets tools from Scrutinizer, uploads unit tests results to Scrutinizer (?)
 after_script:

application/core/Auth.php

@@ -72,7 +72,7 @@ public static function checkAdminAuthentication()
  */
  public static function checkSessionConcurrency(){
  if(Session::userIsLoggedIn()){
- if(Session::isConcurrentSessionExists()){
+ if(Session::isSessionBroken()){
  LoginModel::logout();
  Redirect::home();
  exit();

application/core/Session.php

@@ -84,8 +84,10 @@ public static function updateSessionId($userId, $sessionId = null)
  }
 
  /**
- * checks for session concurrency
- *
+ * checks for broken session 
+ * Session could be broken by Session concurrency or when user is deleted / suspended
+ * 
+ * - Session concurrency is done as the following:
  * This is done as the following:
  * UserA logs in with his session id('123') and it will be stored in the database.
  * Then, UserB logs in also using the same email and password of UserA from another PC,
@@ -94,14 +96,17 @@ public static function updateSessionId($userId, $sessionId = null)
  * Now, Whenever UserA performs any action,
  * You then check the session_id() against the last one stored in the database('456'),
  * If they don't match then log both of them out.
+ * 
+ * - Check for deleted / suspended users:
+ * Suspended/deleted users have no userSessionId anymore stored in database
  *
  * @access public
  * @static static method
  * @return bool
  * @see Session::updateSessionId()
  * @see https://stackoverflow.com/questions/6126285/php-stop-concurrent-user-logins
  */
- public static function isConcurrentSessionExists()
+ public static function isSessionBroken()
  {
  $session_id = session_id();
  $userId = Session::get('user_id');
@@ -117,7 +122,7 @@ public static function isConcurrentSessionExists()
  $result = $query->fetch();
  $userSessionId = !empty($result)? $result->session_id: null;
 
- return $session_id !== $userSessionId;
+ return empty($userSessionId) || $session_id !== $userSessionId;
  }
 
  return false;

tests/core/ConfigTest.php

@@ -23,18 +23,29 @@ public function tearDown()
 
  /**
  * Checks if the correct config file for an EXISTING environment / config is called.
+ * 
+ * @runInSeparateProcess
  */
  public function testGetDefaultEnvironment()
  {
+ // for testing
+ header_remove(); 
+
  // manually set environment to "development"
  putenv('APPLICATION_ENV=development');
 
  // now get the default action to see if the correct config file (for development) is called
  $this->assertEquals('index', Config::get('DEFAULT_ACTION'));
  }
 
+ /**
+ * @runInSeparateProcess
+ */
  public function testGetFailingEnvironment()
  {
+ // for testing
+ header_remove(); 
+
  // manually set environment to "foobar" (and non-existing environment)
  putenv('APPLICATION_ENV=foobar');
 

tests/core/EnvironmentTest.php

@@ -4,14 +4,12 @@ class EnvironmentTest extends PHPUnit_Framework_TestCase
 {
  public function testGetDefault()
  {
- // call for environment should return "development"
- $this->assertEquals('development', Environment::get());
- }
+ // call for environment should return "testing" like set in .travis.yml
+ $this->assertEquals('testing', Environment::get());
 
- public function testGetDevelopment()
- {
- putenv('APPLICATION_ENV=development');
- // call for environment should return "development"
+ putenv('APPLICATION_ENV=');
+
+ // call for environment should now return "development", the default value
  $this->assertEquals('development', Environment::get());
  }
 
@@ -20,4 +18,12 @@ public function testGetProduction()
  putenv('APPLICATION_ENV=production');
  $this->assertEquals('production', Environment::get());
  }
+
+ public function testGetDevelopment()
+ {
+ putenv('APPLICATION_ENV=development');
+ // call for environment should return "development"
+ $this->assertEquals('development', Environment::get());
+ }
+
 }