From 0e9ab7b9ff17172e6220fb32982e02520d1607fb Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 13:30:00 +0200 Subject: [PATCH 1/7] Session validation improvement like proposed her https://github.com/panique/huge/issues/885 --- application/core/Auth.php | 2 +- application/core/Session.php | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/application/core/Auth.php b/application/core/Auth.php index 10430b14a..84761fe7d 100644 --- a/application/core/Auth.php +++ b/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(); diff --git a/application/core/Session.php b/application/core/Session.php index 89d64c6ef..a489a6ec5 100644 --- a/application/core/Session.php +++ b/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,6 +96,9 @@ 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 @@ -101,7 +106,7 @@ public static function updateSessionId($userId, $sessionId = null) * @see Session::updateSessionId() * @see http://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; From a80365662766fe9a67828e2c15b66e4414cc0946 Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 13:30:10 +0200 Subject: [PATCH 2/7] Update ConfigTest.php --- tests/core/ConfigTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/core/ConfigTest.php b/tests/core/ConfigTest.php index 11dae0508..2959e633e 100644 --- a/tests/core/ConfigTest.php +++ b/tests/core/ConfigTest.php @@ -26,6 +26,9 @@ public function tearDown() */ public function testGetDefaultEnvironment() { + // for testing + header_remove(); + // manually set environment to "development" putenv('APPLICATION_ENV=development'); From bb6ed3390b212983d218d8e274a578473ed602e7 Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 13:35:56 +0200 Subject: [PATCH 3/7] Update ConfigTest.php --- tests/core/ConfigTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/core/ConfigTest.php b/tests/core/ConfigTest.php index 2959e633e..fe20047d4 100644 --- a/tests/core/ConfigTest.php +++ b/tests/core/ConfigTest.php @@ -23,6 +23,8 @@ public function tearDown() /** * Checks if the correct config file for an EXISTING environment / config is called. + * + * @runInSeparateProcess */ public function testGetDefaultEnvironment() { @@ -36,8 +38,14 @@ public function testGetDefaultEnvironment() $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'); From f3a04da5701928bf1d3f4d771fa0c82e31dc59b1 Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 13:51:45 +0200 Subject: [PATCH 4/7] Update EnvironmentTest.php --- tests/core/EnvironmentTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/core/EnvironmentTest.php b/tests/core/EnvironmentTest.php index a6d32cf72..8c14d429d 100644 --- a/tests/core/EnvironmentTest.php +++ b/tests/core/EnvironmentTest.php @@ -4,7 +4,10 @@ class EnvironmentTest extends PHPUnit_Framework_TestCase { public function testGetDefault() { - // call for environment should return "development" + // call for environment should return "testing" like set in .travis.yml + $this->assertEquals('testing', Environment::get()); + + putenv('APPLICATION_ENV='); $this->assertEquals('development', Environment::get()); } @@ -20,4 +23,6 @@ public function testGetProduction() putenv('APPLICATION_ENV=production'); $this->assertEquals('production', Environment::get()); } + + } From b6d17ee2223490b41176a9d90cfdc216dd01040d Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 14:00:15 +0200 Subject: [PATCH 5/7] Update EnvironmentTest.php --- tests/core/EnvironmentTest.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/core/EnvironmentTest.php b/tests/core/EnvironmentTest.php index 8c14d429d..462144b85 100644 --- a/tests/core/EnvironmentTest.php +++ b/tests/core/EnvironmentTest.php @@ -8,13 +8,8 @@ public function testGetDefault() $this->assertEquals('testing', Environment::get()); putenv('APPLICATION_ENV='); - $this->assertEquals('development', Environment::get()); - } - public function testGetDevelopment() - { - putenv('APPLICATION_ENV=development'); - // call for environment should return "development" + // call for environment should now return "development", the default value $this->assertEquals('development', Environment::get()); } @@ -24,5 +19,11 @@ public function testGetProduction() $this->assertEquals('production', Environment::get()); } + public function testGetDevelopment() + { + putenv('APPLICATION_ENV=development'); + // call for environment should return "development" + $this->assertEquals('development', Environment::get()); + } } From 5f2e086ef29477ef66c048ebeea276f8b302565d Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 14:11:22 +0200 Subject: [PATCH 6/7] Update .travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3dccb0c09..0fdfc9a7f 100644 --- a/.travis.yml +++ b/.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 --configuration phpunit.xml --coverage-text --coverage-clover=coverage.clover # gets tools from Scrutinizer, uploads unit tests results to Scrutinizer (?) after_script: From 0b646c399265c587e6ddad3061ea02ff7e76a45d Mon Sep 17 00:00:00 2001 From: kristuff Date: Sat, 6 Jun 2020 14:15:19 +0200 Subject: [PATCH 7/7] Update .travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 0fdfc9a7f..9ce1c2496 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,7 +30,7 @@ before_script: - cd tests # run unit tests, create result file -script: ../vendor/bin/phpunit --verbose --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: